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

[PATCH 00/10] 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 for all Samsung
manufactured SoC, we have made changes to Exynos file to extend
support for FSD platform and other Samsung manufactured SoCs which
shall be upstreamed soon.

First a v1 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. This is why not considering it to be v2 of either
patchset.

Currently the DT node is not added in this patchset and will send it
in the devicetree mailing list post this.

Shradha Todi (10):
  PCI: exynos: Change macro names to exynos specific
  PCI: exynos: Remove unused MACROs in exynos PCI file
  PCI: exynos: Reorder MACROs to maintain consistency
  PCI: exynos: Add platform device private data
  PCI: exynos: Add structure to hold resource operations
  dt-bindings: PCI: Add bindings support for Tesla FSD SoC
  dt-bindings: phy: Add PHY bindings support for FSD SoC
  phy: exynos: Add PCIe PHY support for FSD SoC
  PCI: exynos: Add support for Tesla FSD SoC
  misc: pci_endpoint_test: Add driver data for FSD PCIe controllers

 .../bindings/pci/samsung,exynos-pcie-ep.yaml  |  66 ++
 .../bindings/pci/samsung,exynos-pcie.yaml     | 199 +++---
 .../bindings/phy/samsung,exynos-pcie-phy.yaml |   8 +-
 drivers/misc/pci_endpoint_test.c              |   3 +
 drivers/pci/controller/dwc/pci-exynos.c       | 569 +++++++++++++++---
 drivers/phy/samsung/phy-exynos-pcie.c         | 357 ++++++++++-
 include/linux/pci_ids.h                       |   2 +
 7 files changed, 1043 insertions(+), 161 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml

-- 
2.49.0

[PATCH 01/10] 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 | 116 ++++++++++++------------
 1 file changed, 58 insertions(+), 58 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index ace736b025b1..1c70b036376d 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -26,30 +26,30 @@
 #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_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
-#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_LEVEL			0x004
+#define EXYNOS_PCIE_IRQ_SPECIAL		0x008
+#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;
@@ -73,49 +73,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)
@@ -123,21 +123,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)
@@ -150,12 +150,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,
@@ -212,9 +212,9 @@ static struct pci_ops exynos_pci_ops = {
 static int 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 02/10] PCI: exynos: Remove unused MACROs in exynos PCI file

[Shradha Todi]

Some MACROs are defined in the exynos PCI 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 1c70b036376d..990aaa16b132 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -31,8 +31,6 @@
 #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_LEVEL			0x004
-#define EXYNOS_PCIE_IRQ_SPECIAL		0x008
 #define EXYNOS_PCIE_IRQ_EN_PULSE		0x00c
 #define EXYNOS_PCIE_IRQ_EN_LEVEL		0x010
 #define EXYNOS_PCIE_IRQ_EN_SPECIAL		0x014
-- 
2.49.0

Re: [PATCH 02/10] PCI: exynos: Remove unused MACROs in exynos PCI file

[Krzysztof Kozlowski]

On Mon, May 19, 2025 at 01:01:44AM GMT, Shradha Todi wrote:
> Some MACROs are defined in the exynos PCI 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 1c70b036376d..990aaa16b132 100644
> --- a/drivers/pci/controller/dwc/pci-exynos.c
> +++ b/drivers/pci/controller/dwc/pci-exynos.c
> @@ -31,8 +31,6 @@
>  #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_LEVEL			0x004

Fix order of patches. Why renaming something just to remove it? No
point.

Best regards,
Krzysztof

RE: [PATCH 02/10] PCI: exynos: Remove unused MACROs in exynos PCI file

[Shradha Todi]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 21 May 2025 15:11
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.or;
> linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> kw@linux.com; 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; Hrishikesh Dileep <hrishikesh.d@samsung.com>
> Subject: Re: [PATCH 02/10] PCI: exynos: Remove unused MACROs in exynos PCI file
> 
> On Mon, May 19, 2025 at 01:01:44AM GMT, Shradha Todi wrote:
> > Some MACROs are defined in the exynos PCI 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 1c70b036376d..990aaa16b132 100644
> > --- a/drivers/pci/controller/dwc/pci-exynos.c
> > +++ b/drivers/pci/controller/dwc/pci-exynos.c
> > @@ -31,8 +31,6 @@
> >  #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_LEVEL			0x004
> 
> Fix order of patches. Why renaming something just to remove it? No point.
> 

This makes sense. Will change it

> Best regards,
> Krzysztof

[PATCH 03/10] 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 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index 990aaa16b132..286f4987d56f 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,12 +42,12 @@
 #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)
+#define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE		BIT(21)
 
 struct exynos_pcie {
 	struct dw_pcie			pci;
-- 
2.49.0

Re: [PATCH 03/10] PCI: exynos: Reorder MACROs to maintain consistency

[Krzysztof Kozlowski]

On Mon, May 19, 2025 at 01:01:45AM GMT, Shradha Todi wrote:
> 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 | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
> index 990aaa16b132..286f4987d56f 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,12 +42,12 @@
>  #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)
> +#define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE		BIT(21)

What changed here? Why you cannot fix indentation while renaming?

Best regards,
Krzysztof

RE: [PATCH 03/10] PCI: exynos: Reorder MACROs to maintain consistency

[Shradha Todi]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 21 May 2025 15:16
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.or;
> linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> kw@linux.com; 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; Hrishikesh Dileep <hrishikesh.d@samsung.com>
> Subject: Re: [PATCH 03/10] PCI: exynos: Reorder MACROs to maintain consistency
> 
> On Mon, May 19, 2025 at 01:01:45AM GMT, Shradha Todi wrote:
> > 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 | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-exynos.c
> > b/drivers/pci/controller/dwc/pci-exynos.c
> > index 990aaa16b132..286f4987d56f 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,12 +42,12 @@
> >  #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)
> > +#define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE		BIT(21)
> 
> What changed here? Why you cannot fix indentation while renaming?
> 

Will squash the indentation change along with rename

> Best regards,
> Krzysztof

[PATCH 04/10] 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 286f4987d56f..540612e76f4b 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

Re: [PATCH 04/10] PCI: exynos: Add platform device private data

[Krzysztof Kozlowski]

On Mon, May 19, 2025 at 01:01:46AM GMT, Shradha Todi wrote:
> -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);

One more layer of indirection.

Read:
https://lore.kernel.org/all/CAL_JsqJgaeOcnUzw+rUF2yO4hQYCdZYssjxHzrDvvHGJimrASA@mail.gmail.com/

Best regards,
Krzysztof

RE: [PATCH 04/10] PCI: exynos: Add platform device private data

[Shradha Todi]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 21 May 2025 15:15
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.or;
> linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> kw@linux.com; 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 <pankaj.dubey@samsung.com>
> Subject: Re: [PATCH 04/10] PCI: exynos: Add platform device private data
> 
> On Mon, May 19, 2025 at 01:01:46AM GMT, Shradha Todi wrote:
> > -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);
> 
> One more layer of indirection.
> 
> Read:
> https://lore.kernel.org/all/CAL_JsqJgaeOcnUzw+rUF2yO4hQYCdZYssjxHzrDvvHGJimrASA@mail.gmail.com/
> 

I went through this thread and the solution to avoid redirection seems to be:
1. Make the common parts into a library that each driver can call
2. When there is barely anything in common, make a separate driver

From my understanding of these 2 drivers, there is hardly anything that can go into common library
1. host_init, dbi_read, dbi_write these ops have completely different flow
2. link_up, start_link have similar flow but different register offsets
3. write_dbi2 and stop_link is not implemented for exynos but needed for FSD
4. Resources are different - FSD does not have regulator, Exynos5433 does not have syscon, FSD has msi IRQ vs exynos5433 has legacy
5. Exynos is host only whereas FSD is dual mode controller.

I don’t see any other way except redirection, or using lots of if(variant == FSD) which is also discouraged.

And about making it a different driver altogether, I'm completely okay to do so. In fact we had previously tried to post it as a
different driver which was rejected.

If you still feel there is a way to separate out the common parts into a library, please guide me.

> Best regards,
> Krzysztof

Re: [PATCH 04/10] PCI: exynos: Add platform device private data

[Manivannan Sadhasivam]

On Tue, May 27, 2025 at 04:13:58PM +0530, Shradha Todi wrote:
> 
> 
> > -----Original Message-----
> > From: Krzysztof Kozlowski <krzk@kernel.org>
> > Sent: 21 May 2025 15:15
> > To: Shradha Todi <shradha.t@samsung.com>
> > Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.or;
> > linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> > kw@linux.com; 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 <pankaj.dubey@samsung.com>
> > Subject: Re: [PATCH 04/10] PCI: exynos: Add platform device private data
> > 
> > On Mon, May 19, 2025 at 01:01:46AM GMT, Shradha Todi wrote:
> > > -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);
> > 
> > One more layer of indirection.
> > 
> > Read:
> > https://lore.kernel.org/all/CAL_JsqJgaeOcnUzw+rUF2yO4hQYCdZYssjxHzrDvvHGJimrASA@mail.gmail.com/
> > 
> 
> I went through this thread and the solution to avoid redirection seems to be:
> 1. Make the common parts into a library that each driver can call
> 2. When there is barely anything in common, make a separate driver
> 
> From my understanding of these 2 drivers, there is hardly anything that can go into common library
> 1. host_init, dbi_read, dbi_write these ops have completely different flow
> 2. link_up, start_link have similar flow but different register offsets
> 3. write_dbi2 and stop_link is not implemented for exynos but needed for FSD
> 4. Resources are different - FSD does not have regulator, Exynos5433 does not have syscon, FSD has msi IRQ vs exynos5433 has legacy
> 5. Exynos is host only whereas FSD is dual mode controller.
> 
> I don’t see any other way except redirection, or using lots of if(variant == FSD) which is also discouraged.

Please wrap your replies to 80 columns.

> 

IMO it is OK to have the callbacks for cases like this. This is also similar to
Qcom driver where each SoC requires custom register updates and going by the
common library or a new driver wouldn't be feasible.

> And about making it a different driver altogether, I'm completely okay to do so. In fact we had previously tried to post it as a
> different driver which was rejected.
> 

No, please do not create a new driver, that is another maintainer burden.

- Mani

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

[PATCH 05/10] PCI: exynos: Add structure to hold resource operations

[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. Also move common
operations for host init into the probe sequence.

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

diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index 540612e76f4b..b122a2ae8681 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -53,6 +53,7 @@ struct samsung_pcie_pdata {
 	struct pci_ops				*pci_ops;
 	const struct dw_pcie_ops		*dwc_ops;
 	const struct dw_pcie_host_ops		*host_ops;
+	const struct samsung_res_ops		*res_ops;
 };
 
 struct exynos_pcie {
@@ -61,7 +62,13 @@ 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);
 };
 
 static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
@@ -74,6 +81,36 @@ static u32 exynos_pcie_readl(void __iomem *base, u32 reg)
 	return readl(base + reg);
 }
 
+static int samsung_regulator_enable(struct exynos_pcie *ep)
+{
+	struct device *dev = ep->pci.dev;
+	int ret;
+
+	if (ep->supplies_cnt == 0)
+		return 0;
+
+	ret = devm_regulator_bulk_get(dev, ep->supplies_cnt, ep->supplies);
+	if (ret)
+		return ret;
+
+	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 +281,23 @@ 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;
+
+	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";
+
+	return 0;
+}
+
+static int samsung_irq_init(struct exynos_pcie *ep,
 				       struct platform_device *pdev)
 {
 	struct dw_pcie *pci = &ep->pci;
@@ -256,22 +309,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 +328,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 +364,31 @@ 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);
+	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_probe;
+		goto fail_phy_init;
 
 	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;
 }
@@ -347,7 +401,7 @@ static void exynos_pcie_remove(struct platform_device *pdev)
 	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)
@@ -357,7 +411,7 @@ static int exynos_pcie_suspend_noirq(struct device *dev)
 	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 +423,7 @@ 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);
+	ret = samsung_regulator_enable(ep);
 	if (ret)
 		return ret;
 
@@ -389,6 +443,7 @@ static const struct samsung_pcie_pdata exynos_5433_pcie_rc_pdata = {
 	.dwc_ops		= &exynos_dw_pcie_ops,
 	.pci_ops		= &exynos_pci_ops,
 	.host_ops		= &exynos_pcie_host_ops,
+	.res_ops		= &exynos_res_ops_data,
 };
 
 static const struct of_device_id exynos_pcie_of_match[] = {
-- 
2.49.0

Re: [PATCH 05/10] PCI: exynos: Add structure to hold resource operations

[Krzysztof Kozlowski]

On Mon, May 19, 2025 at 01:01:47AM GMT, Shradha Todi wrote:
> +struct samsung_res_ops {
> +	int (*init_regulator)(struct exynos_pcie *ep);
> +	irqreturn_t (*pcie_irq_handler)(int irq, void *arg);
>  };
>  
>  static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
> @@ -74,6 +81,36 @@ static u32 exynos_pcie_readl(void __iomem *base, u32 reg)
>  	return readl(base + reg);
>  }
>  
> +static int samsung_regulator_enable(struct exynos_pcie *ep)
> +{
> +	struct device *dev = ep->pci.dev;
> +	int ret;
> +
> +	if (ep->supplies_cnt == 0)
> +		return 0;
> +
> +	ret = devm_regulator_bulk_get(dev, ep->supplies_cnt, ep->supplies);

No. Getting resources on every enable is making this much less readable.

NAK

Best regards,
Krzysztof

RE: [PATCH 05/10] PCI: exynos: Add structure to hold resource operations

[Shradha Todi]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 21 May 2025 15:13
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.or;
> linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> kw@linux.com; 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 <pankaj.dubey@samsung.com>
> Subject: Re: [PATCH 05/10] PCI: exynos: Add structure to hold resource operations
> 
> On Mon, May 19, 2025 at 01:01:47AM GMT, Shradha Todi wrote:
> > +struct samsung_res_ops {
> > +	int (*init_regulator)(struct exynos_pcie *ep);
> > +	irqreturn_t (*pcie_irq_handler)(int irq, void *arg);
> >  };
> >
> >  static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
> > @@ -74,6 +81,36 @@ static u32 exynos_pcie_readl(void __iomem *base, u32 reg)
> >  	return readl(base + reg);
> >  }
> >
> > +static int samsung_regulator_enable(struct exynos_pcie *ep) {
> > +	struct device *dev = ep->pci.dev;
> > +	int ret;
> > +
> > +	if (ep->supplies_cnt == 0)
> > +		return 0;
> > +
> > +	ret = devm_regulator_bulk_get(dev, ep->supplies_cnt, ep->supplies);
> 
> No. Getting resources on every enable is making this much less readable.
> 
> NAK
> 

Will make sure that we get the resources only once during probe.

> Best regards,
> Krzysztof

[PATCH 06/10] dt-bindings: PCI: Add bindings support for Tesla FSD SoC

[Shradha Todi]

Document the PCIe controller device tree bindings for Tesla FSD
SoC for both RC and EP.

Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 .../bindings/pci/samsung,exynos-pcie-ep.yaml  |  66 ++++++
 .../bindings/pci/samsung,exynos-pcie.yaml     | 199 ++++++++++++------
 2 files changed, 198 insertions(+), 67 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml

diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml
new file mode 100644
index 000000000000..5d4a9067f727
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/samsung,exynos-pcie-ep.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung SoC series PCIe Endpoint Controller
+
+maintainers:
+  - Shradha Todi <shradha.t@samsung.co>
+
+description: |+
+  Samsung 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.
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - tesla,fsd-pcie-ep
+
+allOf:
+  - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - tesla,fsd-pcie-ep
+    then:
+      properties:
+        samsung,syscon-pcie:
+          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>
+    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>;
+            samsung,syscon-pcie = <&sysreg_fsys1 0x50c>;
+            phys = <&pciephy1>;
+        };
+    };
+...
diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
index f20ed7e709f7..a3803bf0ef84 100644
--- a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
@@ -11,78 +11,113 @@ 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.
 
-allOf:
-  - $ref: /schemas/pci/snps,dw-pcie.yaml#
-
 properties:
   compatible:
-    const: samsung,exynos5433-pcie
-
-  reg:
-    items:
-      - description: Data Bus Interface (DBI) registers.
-      - description: External Local Bus interface (ELBI) registers.
-      - description: PCIe configuration space region.
-
-  reg-names:
-    items:
-      - const: dbi
-      - const: elbi
-      - const: config
-
-  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
+    oneOf:
+      - enum:
+          - samsung,exynos5433-pcie
+          - tesla,fsd-pcie
+
+allOf:
+  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - tesla,fsd-pcie
+    then:
+      properties:
+        samsung,syscon-pcie:
+          description: phandle for system control registers, used to
+                       control signals at system level
+
+      required:
+        - samsung,syscon-pcie
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - samsung,exynos5433-pcie
+    then:
+      properties:
+        reg:
+          items:
+            - description: controller's own configuration registers
+                           are available.
+            - description: controller's application logic registers
+            - description: configuration registers
+
+        reg-names:
+          items:
+            - const: dbi
+            - const: elbi
+            - const: config
+
+        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
+
+        assigned-clocks:
+          maxItems: 2
+
+        assigned-clock-parents:
+          maxItems: 2
+
+        assigned-clock-rates:
+          maxItems: 2
+
+      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
 
@@ -116,4 +151,34 @@ examples:
         interrupt-map-mask = <0 0 0 0>;
         interrupt-map = <0 0 0 0 &gic GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
     };
+  - |
+    #include <dt-bindings/clock/fsd-clk.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    bus {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        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>;
+            samsung,syscon-pcie = <&sysreg_fsys1 0x50c>;
+            phys = <&pciephy1>;
+            iommu-map = <0x0 &smmu_imem 0x0 0x10000>;
+            iommu-map-mask = <0x0>;
+        };
+    };
 ...
-- 
2.49.0

Re: [PATCH 06/10] dt-bindings: PCI: Add bindings support for Tesla FSD SoC

[Krzysztof Kozlowski]

On Mon, May 19, 2025 at 01:01:48AM GMT, Shradha Todi wrote:
> Document the PCIe controller device tree bindings for Tesla FSD
> SoC for both RC and EP.
> 
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  .../bindings/pci/samsung,exynos-pcie-ep.yaml  |  66 ++++++
>  .../bindings/pci/samsung,exynos-pcie.yaml     | 199 ++++++++++++------
>  2 files changed, 198 insertions(+), 67 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml
> new file mode 100644
> index 000000000000..5d4a9067f727
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml

Filename matching compatible.

> @@ -0,0 +1,66 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/samsung,exynos-pcie-ep.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung SoC series PCIe Endpoint Controller
> +
> +maintainers:
> +  - Shradha Todi <shradha.t@samsung.co>
> +
> +description: |+
> +  Samsung 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.
> +
> +properties:
> +  compatible:
> +    oneOf:

Drop

> +      - enum:
> +          - tesla,fsd-pcie-ep
> +
> +allOf:
> +  - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - tesla,fsd-pcie-ep

What is the point of this if:? There are no other variants.

Also, missing constraints for all the properties. This is really
incomplete.

> +    then:
> +      properties:
> +        samsung,syscon-pcie:
> +          description: phandle for system control registers, used to
> +                       control signals at system level

Where is the type defined? Look how such properties are described -
there are plenty of examples.

> +
> +      required:
> +        - 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>;
> +            samsung,syscon-pcie = <&sysreg_fsys1 0x50c>;
> +            phys = <&pciephy1>;
> +        };
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> index f20ed7e709f7..a3803bf0ef84 100644
> --- a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> @@ -11,78 +11,113 @@ 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.
>  
> -allOf:
> -  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> -
>  properties:
>    compatible:
> -    const: samsung,exynos5433-pcie
> -
> -  reg:
> -    items:
> -      - description: Data Bus Interface (DBI) registers.
> -      - description: External Local Bus interface (ELBI) registers.
> -      - description: PCIe configuration space region.
> -

No, I do not understand any of this change. Properties are defined in
top-level. Why all this is being removed?


Best regards,
Krzysztof

RE: [PATCH 06/10] dt-bindings: PCI: Add bindings support for Tesla FSD SoC

[Shradha Todi]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 21 May 2025 15:07
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.or;
> linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> kw@linux.com; 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
> Subject: Re: [PATCH 06/10] dt-bindings: PCI: Add bindings support for Tesla FSD SoC
> 
> On Mon, May 19, 2025 at 01:01:48AM GMT, Shradha Todi wrote:
> > Document the PCIe controller device tree bindings for Tesla FSD SoC
> > for both RC and EP.
> >
> > Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> > ---
> >  .../bindings/pci/samsung,exynos-pcie-ep.yaml  |  66 ++++++
> >  .../bindings/pci/samsung,exynos-pcie.yaml     | 199 ++++++++++++------
> >  2 files changed, 198 insertions(+), 67 deletions(-)  create mode
> > 100644
> > Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml
> > b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yaml
> > new file mode 100644
> > index 000000000000..5d4a9067f727
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie-ep.yam
> > +++ l
> 
> Filename matching compatible.

Okay, will change it to tesla,fsd-pcie-ep.yaml

> 
> > @@ -0,0 +1,66 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://protect2.fireeye.com/v1/url?k=011d92c7-5e86abcb-011c1988-000b
> > +abff3563-f87bc6d1cb527c28&q=1&e=3d0e8e81-bcdc-412b-ba41-5d5936c37c73&
> > +u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fpci%2Fsamsung%2Cexynos-pcie
> > +-ep.yaml%23
> > +$schema:
> > +https://protect2.fireeye.com/v1/url?k=dc0b3b6d-83900261-dc0ab022-000b
> > +abff3563-91c2c3470c50d358&q=1&e=3d0e8e81-bcdc-412b-ba41-5d5936c37c73&
> > +u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
> > +
> > +title: Samsung SoC series PCIe Endpoint Controller
> > +
> > +maintainers:
> > +  - Shradha Todi <shradha.t@samsung.co>
> > +
> > +description: |+
> > +  Samsung 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.
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> 
> Drop
> 
> > +      - enum:
> > +          - tesla,fsd-pcie-ep
> > +
> > +allOf:
> > +  - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - tesla,fsd-pcie-ep
> 
> What is the point of this if:? There are no other variants.
> 
> Also, missing constraints for all the properties. This is really incomplete.
> 

Will add the constraints

> > +    then:
> > +      properties:
> > +        samsung,syscon-pcie:
> > +          description: phandle for system control registers, used to
> > +                       control signals at system level
> 
> Where is the type defined? Look how such properties are described - there are plenty of examples.
> 
> > +
> > +      required:
> > +        - 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>;
> > +            samsung,syscon-pcie = <&sysreg_fsys1 0x50c>;
> > +            phys = <&pciephy1>;
> > +        };
> > +    };
> > +...
> > diff --git
> > a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> > b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> > index f20ed7e709f7..a3803bf0ef84 100644
> > --- a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> > @@ -11,78 +11,113 @@ 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.
> >
> > -allOf:
> > -  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > -
> >  properties:
> >    compatible:
> > -    const: samsung,exynos5433-pcie
> > -
> > -  reg:
> > -    items:
> > -      - description: Data Bus Interface (DBI) registers.
> > -      - description: External Local Bus interface (ELBI) registers.
> > -      - description: PCIe configuration space region.
> > -
> 
> No, I do not understand any of this change. Properties are defined in top-level. Why all this is being removed?
> 

I changed the binding file to include both FSD and exynos which have quite a few different DT properties and constraints. I understand
I should keep the common properties like reg, phys defined in top-level. Will do that.

> 
> Best regards,
> Krzysztof

[PATCH 07/10] dt-bindings: phy: Add PHY bindings support for FSD SoC

[Shradha Todi]

Document PHY device tree bindings for Tesla FSD SoCs.

Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 .../devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml  | 8 ++++++--
 1 file changed, 6 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..3a5bff0fb82d 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
+    oneOf:
+      - enum:
+          - samsung,exynos5433-pcie-phy
+          - tesla,fsd-pcie-phy
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   samsung,pmu-syscon:
     $ref: /schemas/types.yaml#/definitions/phandle
-- 
2.49.0

Re: [PATCH 07/10] dt-bindings: phy: Add PHY bindings support for FSD SoC

[Krzysztof Kozlowski]

On Mon, May 19, 2025 at 01:01:49AM GMT, Shradha Todi wrote:
> Document PHY device tree bindings for Tesla FSD SoCs.
> 
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  .../devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml  | 8 ++++++--
>  1 file changed, 6 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..3a5bff0fb82d 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
> +    oneOf:

Drop, that's just enumm unless you already add here more?

> +      - enum:
> +          - samsung,exynos5433-pcie-phy
> +          - tesla,fsd-pcie-phy
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2

You need to list the items and constrain existing variants. I do not get
why exynos5433 gets now two MMIO ranges.

Best regards,
Krzysztof

RE: [PATCH 07/10] dt-bindings: phy: Add PHY bindings support for FSD SoC

[Shradha Todi]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 21 May 2025 15:03
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.or;
> linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> kw@linux.com; 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
> Subject: Re: [PATCH 07/10] dt-bindings: phy: Add PHY bindings support for FSD SoC
> 
> On Mon, May 19, 2025 at 01:01:49AM GMT, Shradha Todi wrote:
> > Document PHY device tree bindings for Tesla FSD SoCs.
> >
> > Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> > ---
> >  .../devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml  | 8
> > ++++++--
> >  1 file changed, 6 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..3a5bff0fb82d 100644
> > ---
> > a/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.ya
> > +++ ml
> > @@ -15,10 +15,14 @@ properties:
> >      const: 0
> >
> >    compatible:
> > -    const: samsung,exynos5433-pcie-phy
> > +    oneOf:
> 
> Drop, that's just enumm unless you already add here more?
> 
> > +      - enum:
> > +          - samsung,exynos5433-pcie-phy
> > +          - tesla,fsd-pcie-phy
> >
> >    reg:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 2
> 
> You need to list the items and constrain existing variants. I do not get why exynos5433 gets now two MMIO ranges.
> 

Will constraint both variants with if - else

> Best regards,
> Krzysztof

[PATCH 08/10] 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 | 357 +++++++++++++++++++++++++-
 1 file changed, 356 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/samsung/phy-exynos-pcie.c b/drivers/phy/samsung/phy-exynos-pcie.c
index 53c9230c2907..0e4c00c1121e 100644
--- a/drivers/phy/samsung/phy-exynos-pcie.c
+++ b/drivers/phy/samsung/phy-exynos-pcie.c
@@ -34,11 +34,121 @@
 /* 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_PHY_LANE_OFFSET	0x400
+#define FSD_PCIE_PHY_TRSV_REG001_LN_N	0x404
+#define FSD_PCIE_PHY_TRSV_REG002_LN_N	0x408
+#define FSD_PCIE_PHY_TRSV_REG005_LN_N	0x414
+#define FSD_PCIE_PHY_TRSV_REG006_LN_N	0x418
+#define FSD_PCIE_PHY_TRSV_REG007_LN_N	0x41c
+#define FSD_PCIE_PHY_TRSV_REG009_LN_N	0x424
+#define FSD_PCIE_PHY_TRSV_REG00A_LN_N	0x428
+#define FSD_PCIE_PHY_TRSV_REG00C_LN_N	0x430
+#define FSD_PCIE_PHY_TRSV_REG012_LN_N	0x448
+#define FSD_PCIE_PHY_TRSV_REG013_LN_N	0x44c
+#define FSD_PCIE_PHY_TRSV_REG014_LN_N	0x450
+#define FSD_PCIE_PHY_TRSV_REG015_LN_N	0x454
+#define FSD_PCIE_PHY_TRSV_REG016_LN_N	0x458
+#define FSD_PCIE_PHY_TRSV_REG018_LN_N	0x460
+#define FSD_PCIE_PHY_TRSV_REG020_LN_N	0x480
+#define FSD_PCIE_PHY_TRSV_REG026_LN_N	0x498
+#define FSD_PCIE_PHY_TRSV_REG029_LN_N	0x4a4
+#define FSD_PCIE_PHY_TRSV_REG031_LN_N	0x4c4
+#define FSD_PCIE_PHY_TRSV_REG036_LN_N	0x4d8
+#define FSD_PCIE_PHY_TRSV_REG039_LN_N	0x4e4
+#define FSD_PCIE_PHY_TRSV_REG03B_LN_N	0x4ec
+#define FSD_PCIE_PHY_TRSV_REG03C_LN_N	0x4f0
+#define FSD_PCIE_PHY_TRSV_REG03E_LN_N	0x4f8
+#define FSD_PCIE_PHY_TRSV_REG03F_LN_N	0x4fc
+#define FSD_PCIE_PHY_TRSV_REG043_LN_N	0x50c
+#define FSD_PCIE_PHY_TRSV_REG044_LN_N	0x510
+#define FSD_PCIE_PHY_TRSV_REG046_LN_N	0x518
+#define FSD_PCIE_PHY_TRSV_REG048_LN_N	0x520
+#define FSD_PCIE_PHY_TRSV_REG049_LN_N	0x524
+#define FSD_PCIE_PHY_TRSV_REG04E_LN_N	0x538
+#define FSD_PCIE_PHY_TRSV_REG052_LN_N	0x548
+#define FSD_PCIE_PHY_TRSV_REG068_LN_N	0x5a0
+#define FSD_PCIE_PHY_TRSV_REG069_LN_N	0x5a4
+#define FSD_PCIE_PHY_TRSV_REG06A_LN_N	0x5a8
+#define FSD_PCIE_PHY_TRSV_REG06B_LN_N	0x5ac
+#define FSD_PCIE_PHY_TRSV_REG07B_LN_N	0x5ec
+#define FSD_PCIE_PHY_TRSV_REG083_LN_N	0x60c
+#define FSD_PCIE_PHY_TRSV_REG084_LN_N	0x610
+#define FSD_PCIE_PHY_TRSV_REG086_LN_N	0x618
+#define FSD_PCIE_PHY_TRSV_REG087_LN_N	0x61c
+#define FSD_PCIE_PHY_TRSV_REG08B_LN_N	0x62c
+#define FSD_PCIE_PHY_TRSV_REG09C_LN_N	0x670
+#define FSD_PCIE_PHY_TRSV_REG09D_LN_N	0x674
+#define FSD_PCIE_PHY_TRSV_REG09E_LN_N	0x678
+#define FSD_PCIE_PHY_TRSV_REG09F_LN_N	0x67c
+#define FSD_PCIE_PHY_TRSV_REG0A2_LN_N	0x688
+#define FSD_PCIE_PHY_TRSV_REG0A4_LN_N	0x690
+#define FSD_PCIE_PHY_TRSV_REG0CE_LN_N	0x738
+#define FSD_PCIE_PHY_TRSV_REG0FC_LN_N	0x7f0
+#define FSD_PCIE_PHY_TRSV_REG0FD_LN_N	0x7f4
+#define FSD_PCIE_PHY_TRSV_REG0FE_LN_N	0x7f8
+#define FSD_PCIE_PHY_TRSV_REG0CE_LN_1	0xb38
+#define FSD_PCIE_PHY_TRSV_REG0CE_LN_2	0xf38
+#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_MASK			0x3ff
+#define FSD_PCIE_SYSREG_PHY_0_CON			0x042C
+#define FSD_PCIE_SYSREG_PHY_0_REF_SEL_MASK		0x3
+#define FSD_PCIE_SYSREG_PHY_0_REF_SEL			(0x2 << 0)
+#define FSD_PCIE_SYSREG_PHY_0_SSC_EN_MASK		0x8
+#define FSD_PCIE_SYSREG_PHY_0_SSC_EN			BIT(3)
+#define FSD_PCIE_SYSREG_PHY_0_AUX_EN_MASK		0x10
+#define FSD_PCIE_SYSREG_PHY_0_AUX_EN			BIT(4)
+#define FSD_PCIE_SYSREG_PHY_0_CMN_RSTN_MASK		0x100
+#define FSD_PCIE_SYSREG_PHY_0_CMN_RSTN			BIT(8)
+#define FSD_PCIE_SYSREG_PHY_0_INIT_RSTN_MASK		0x200
+#define FSD_PCIE_SYSREG_PHY_0_INIT_RSTN			BIT(9)
+
+#define FSD_PCIE_SYSREG_PHY_1_CON_MASK			0x1ff
+#define FSD_PCIE_SYSREG_PHY_1_CON			0x0500
+#define FSD_PCIE_SYSREG_PHY_1_REF_SEL_MASK		0x30
+#define FSD_PCIE_SYSREG_PHY_1_REF_SEL			(0x2 << 4)
+#define FSD_PCIE_SYSREG_PHY_1_SSC_EN_MASK		0x80
+#define FSD_PCIE_SYSREG_PHY_1_SSC_EN			BIT(7)
+#define FSD_PCIE_SYSREG_PHY_1_AUX_EN_MASK		0x1
+#define FSD_PCIE_SYSREG_PHY_1_AUX_EN			BIT(0)
+#define FSD_PCIE_SYSREG_PHY_1_CMN_RSTN_MASK		0x2
+#define FSD_PCIE_SYSREG_PHY_1_CMN_RSTN			BIT(1)
+#define FSD_PCIE_SYSREG_PHY_1_INIT_RSTN_MASK		0x8
+#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;
+	int phy_id;
+	const struct samsung_drv_data *drv_data;
+};
+
+struct samsung_drv_data {
+	const struct phy_ops *phy_ops;
 };
 
 static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
@@ -133,9 +243,244 @@ static const struct phy_ops exynos5433_phy_ops = {
 	.owner		= THIS_MODULE,
 };
 
+struct fsd_pcie_phy_pdata {
+	u32 phy_con_mask;
+	u32 phy_con;
+	u32 phy_ref_sel_mask;
+	u32 phy_ref_sel;
+	u32 phy_ssc_en_mask;
+	u32 phy_ssc_en;
+	u32 phy_aux_en_mask;
+	u32 phy_aux_en;
+	u32 phy_cmn_rstn_mask;
+	u32 phy_cmn_rstn;
+	u32 phy_init_rstn_mask;
+	u32 phy_init_rstn;
+	u32 num_lanes;
+	u32 lane_offset;
+};
+
+struct fsd_pcie_phy_pdata fsd_phy_con[] = {
+	{
+	.phy_con		= FSD_PCIE_SYSREG_PHY_0_CON,
+	.phy_con_mask		= FSD_PCIE_SYSREG_PHY_0_CON_MASK,
+	.phy_ref_sel_mask	= FSD_PCIE_SYSREG_PHY_0_REF_SEL_MASK,
+	.phy_ref_sel		= FSD_PCIE_SYSREG_PHY_0_REF_SEL,
+	.phy_ssc_en_mask	= FSD_PCIE_SYSREG_PHY_0_SSC_EN_MASK,
+	.phy_ssc_en		= FSD_PCIE_SYSREG_PHY_0_SSC_EN,
+	.phy_aux_en_mask	= FSD_PCIE_SYSREG_PHY_0_AUX_EN_MASK,
+	.phy_aux_en		= FSD_PCIE_SYSREG_PHY_0_AUX_EN,
+	.phy_cmn_rstn_mask	= FSD_PCIE_SYSREG_PHY_0_CMN_RSTN_MASK,
+	.phy_cmn_rstn		= FSD_PCIE_SYSREG_PHY_0_CMN_RSTN,
+	.phy_init_rstn_mask	= FSD_PCIE_SYSREG_PHY_0_INIT_RSTN_MASK,
+	.phy_init_rstn		= FSD_PCIE_SYSREG_PHY_0_INIT_RSTN,
+	.num_lanes		= 0x4,
+	.lane_offset		= FSD_PCIE_PHY_LANE_OFFSET,
+	},
+	{
+	.phy_con		= FSD_PCIE_SYSREG_PHY_1_CON,
+	.phy_con_mask		= FSD_PCIE_SYSREG_PHY_1_CON_MASK,
+	.phy_ref_sel_mask	= FSD_PCIE_SYSREG_PHY_1_REF_SEL_MASK,
+	.phy_ref_sel		= FSD_PCIE_SYSREG_PHY_1_REF_SEL,
+	.phy_ssc_en_mask	= FSD_PCIE_SYSREG_PHY_1_SSC_EN_MASK,
+	.phy_ssc_en		= FSD_PCIE_SYSREG_PHY_1_SSC_EN,
+	.phy_aux_en_mask	= FSD_PCIE_SYSREG_PHY_1_AUX_EN_MASK,
+	.phy_aux_en		= FSD_PCIE_SYSREG_PHY_1_AUX_EN,
+	.phy_cmn_rstn_mask	= FSD_PCIE_SYSREG_PHY_1_CMN_RSTN_MASK,
+	.phy_cmn_rstn		= FSD_PCIE_SYSREG_PHY_1_CMN_RSTN,
+	.phy_init_rstn_mask	= FSD_PCIE_SYSREG_PHY_1_INIT_RSTN_MASK,
+	.phy_init_rstn		= FSD_PCIE_SYSREG_PHY_1_INIT_RSTN,
+	.num_lanes		= 0x4,
+	.lane_offset		= FSD_PCIE_PHY_LANE_OFFSET,
+	},
+	{ },
+};
+
+struct fsd_pcie_phy_setting {
+	u32 addr;
+	u32 val;
+	bool is_cmn_reg;
+};
+
+struct fsd_pcie_phy_setting fsd_pcie_phy0_setting[] = {
+	{FSD_PCIE_PHY_TRSV_REG07B_LN_N, 0x20, false},
+	{FSD_PCIE_PHY_TRSV_REG052_LN_N, 0x00, false},
+	{FSD_PCIE_PHY_TRSV_CMN_REG05F, 0x11, true},
+	{FSD_PCIE_PHY_TRSV_CMN_REG060, 0x23, true},
+	{FSD_PCIE_PHY_TRSV_CMN_REG062, 0x0, true},
+	{FSD_PCIE_PHY_TRSV_CMN_REG03, 0x15, true},
+	{FSD_PCIE_PHY_TRSV_REG0CE_LN_N, 0x8, false},
+	{FSD_PCIE_PHY_TRSV_REG039_LN_N, 0xf, false},
+	{FSD_PCIE_PHY_TRSV_REG03B_LN_N, 0x13, false},
+	{FSD_PCIE_PHY_TRSV_REG03C_LN_N, 0xf6, false},
+	{FSD_PCIE_PHY_TRSV_REG044_LN_N, 0x57, false},
+	{FSD_PCIE_PHY_TRSV_REG03E_LN_N, 0x10, false},
+	{FSD_PCIE_PHY_TRSV_REG03F_LN_N, 0x04, false},
+	{FSD_PCIE_PHY_TRSV_REG043_LN_N, 0x11, false},
+	{FSD_PCIE_PHY_TRSV_REG049_LN_N, 0x6f, false},
+	{FSD_PCIE_PHY_TRSV_REG04E_LN_N, 0x18, false},
+	{FSD_PCIE_PHY_TRSV_REG068_LN_N, 0x1f, false},
+	{FSD_PCIE_PHY_TRSV_REG069_LN_N, 0xc, false},
+	{FSD_PCIE_PHY_TRSV_REG06B_LN_N, 0x78, false},
+	{FSD_PCIE_PHY_TRSV_REG083_LN_N, 0xa, false},
+	{FSD_PCIE_PHY_TRSV_REG084_LN_N, 0x80, false},
+	{FSD_PCIE_PHY_TRSV_REG086_LN_N, 0xff, false},
+	{FSD_PCIE_PHY_TRSV_REG087_LN_N, 0x3c, false},
+	{FSD_PCIE_PHY_TRSV_REG09D_LN_N, 0x7c, false},
+	{FSD_PCIE_PHY_TRSV_REG09E_LN_N, 0x33, false},
+	{FSD_PCIE_PHY_TRSV_REG09F_LN_N, 0x33, false},
+	{FSD_PCIE_PHY_TRSV_REG001_LN_N, 0x3f, false},
+	{FSD_PCIE_PHY_TRSV_REG002_LN_N, 0x1c, false},
+	{FSD_PCIE_PHY_TRSV_REG005_LN_N, 0x2b, false},
+	{FSD_PCIE_PHY_TRSV_REG006_LN_N, 0x3, false},
+	{FSD_PCIE_PHY_TRSV_REG007_LN_N, 0x0c, false},
+	{FSD_PCIE_PHY_TRSV_REG009_LN_N, 0x10, false},
+	{FSD_PCIE_PHY_TRSV_REG00A_LN_N, 0x1, false},
+	{FSD_PCIE_PHY_TRSV_REG00C_LN_N, 0x93, false},
+	{FSD_PCIE_PHY_TRSV_REG012_LN_N, 0x1, false},
+	{FSD_PCIE_PHY_TRSV_REG013_LN_N, 0x0, false},
+	{FSD_PCIE_PHY_TRSV_REG014_LN_N, 0x70, false},
+	{FSD_PCIE_PHY_TRSV_REG015_LN_N, 0x0, false},
+	{FSD_PCIE_PHY_TRSV_REG016_LN_N, 0x70, false},
+	{FSD_PCIE_PHY_TRSV_REG0FC_LN_N, 0x80, false},
+	{FSD_PCIE_PHY_TRSV_REG0FD_LN_N, 0x0, false},
+};
+
+struct fsd_pcie_phy_setting fsd_pcie_phy1_setting[] = {
+	{FSD_PCIE_PHY_TRSV_REG07B_LN_N, 0x20, false},
+	{FSD_PCIE_PHY_TRSV_REG052_LN_N, 0x00, false},
+	{FSD_PCIE_PHY_TRSV_CMN_REG01E, 0xaa, true},
+	{FSD_PCIE_PHY_TRSV_CMN_REG02D, 0x28, true},
+	{FSD_PCIE_PHY_TRSV_CMN_REG031, 0x28, true},
+	{FSD_PCIE_PHY_TRSV_CMN_REG036, 0x21, true},
+	{FSD_PCIE_PHY_TRSV_CMN_REG05F, 0x12, true},
+	{FSD_PCIE_PHY_TRSV_CMN_REG060, 0x23, true},
+	{FSD_PCIE_PHY_TRSV_CMN_REG061, 0x0, true},
+	{FSD_PCIE_PHY_TRSV_CMN_REG062, 0x0, true},
+	{FSD_PCIE_PHY_TRSV_CMN_REG03, 0x15, true},
+	{FSD_PCIE_PHY_TRSV_REG039_LN_N, 0xf, false},
+	{FSD_PCIE_PHY_TRSV_REG03B_LN_N, 0x13, false},
+	{FSD_PCIE_PHY_TRSV_REG03C_LN_N, 0x66, false},
+	{FSD_PCIE_PHY_TRSV_REG044_LN_N, 0x57, false},
+	{FSD_PCIE_PHY_TRSV_REG03E_LN_N, 0x10, false},
+	{FSD_PCIE_PHY_TRSV_REG03F_LN_N, 0x44, false},
+	{FSD_PCIE_PHY_TRSV_REG043_LN_N, 0x11, false},
+	{FSD_PCIE_PHY_TRSV_REG046_LN_N, 0xef, false},
+	{FSD_PCIE_PHY_TRSV_REG048_LN_N, 0x06, false},
+	{FSD_PCIE_PHY_TRSV_REG049_LN_N, 0xaf, false},
+	{FSD_PCIE_PHY_TRSV_REG04E_LN_N, 0x28, false},
+	{FSD_PCIE_PHY_TRSV_REG068_LN_N, 0x1f, false},
+	{FSD_PCIE_PHY_TRSV_REG069_LN_N, 0xc, false},
+	{FSD_PCIE_PHY_TRSV_REG06A_LN_N, 0x8, false},
+	{FSD_PCIE_PHY_TRSV_REG06B_LN_N, 0x78, false},
+	{FSD_PCIE_PHY_TRSV_REG083_LN_N, 0xa, false},
+	{FSD_PCIE_PHY_TRSV_REG084_LN_N, 0x80, false},
+	{FSD_PCIE_PHY_TRSV_REG087_LN_N, 0x30, false},
+	{FSD_PCIE_PHY_TRSV_REG08B_LN_N, 0xa0, false},
+	{FSD_PCIE_PHY_TRSV_REG09C_LN_N, 0xf7, false},
+	{FSD_PCIE_PHY_TRSV_REG09E_LN_N, 0x33, false},
+	{FSD_PCIE_PHY_TRSV_REG0A2_LN_N, 0xfa, false},
+	{FSD_PCIE_PHY_TRSV_REG0A4_LN_N, 0xf2, false},
+	{FSD_PCIE_PHY_TRSV_REG0CE_LN_N, 0x08, true},
+	{FSD_PCIE_PHY_TRSV_REG0CE_LN_1, 0x09, true},
+	{FSD_PCIE_PHY_TRSV_REG0CE_LN_2, 0x09, true},
+	{FSD_PCIE_PHY_TRSV_REG0CE_LN_3, 0x09, true},
+	{FSD_PCIE_PHY_TRSV_REG0FE_LN_N, 0x33, false},
+	{FSD_PCIE_PHY_TRSV_REG001_LN_N, 0x3f, false},
+	{FSD_PCIE_PHY_TRSV_REG005_LN_N, 0x2b, false},
+};
+
+static void fsd_pcie_phy_writel(struct exynos_pcie_phy *phy_ctrl,
+							u32 val, u32 offset)
+{
+	struct fsd_pcie_phy_pdata *pdata = &fsd_phy_con[phy_ctrl->phy_id];
+	void __iomem *phy_base = phy_ctrl->base;
+	u32 i;
+
+	for (i = 0; i < pdata->num_lanes; i++)
+		writel(val, phy_base + (offset + i * pdata->lane_offset));
+}
+
+static int fsd_pcie_phy_reset(struct phy *phy)
+{
+	struct exynos_pcie_phy *phy_ctrl = phy_get_drvdata(phy);
+	struct fsd_pcie_phy_pdata *pdata = &fsd_phy_con[phy_ctrl->phy_id];
+
+	writel(0x1, phy_ctrl->pcs_base + FSD_PCIE_PCS_CLK);
+
+	regmap_update_bits(phy_ctrl->fsysreg, pdata->phy_con, pdata->phy_con_mask,
+			   0x0);
+	regmap_update_bits(phy_ctrl->fsysreg, pdata->phy_con, pdata->phy_aux_en_mask,
+			   pdata->phy_aux_en);
+	regmap_update_bits(phy_ctrl->fsysreg, pdata->phy_con, pdata->phy_ref_sel_mask,
+			   pdata->phy_ref_sel);
+
+	/* perform init reset release */
+	regmap_update_bits(phy_ctrl->fsysreg, pdata->phy_con,
+			pdata->phy_init_rstn_mask, pdata->phy_init_rstn);
+	return 0;
+}
+
+static int fsd_pcie_phy_init(struct phy *phy)
+{
+	struct fsd_pcie_phy_setting *phy_param = fsd_pcie_phy0_setting;
+	struct exynos_pcie_phy *phy_ctrl = phy_get_drvdata(phy);
+	struct fsd_pcie_phy_pdata *pdata = &fsd_phy_con[phy_ctrl->phy_id];
+	int len = ARRAY_SIZE(fsd_pcie_phy0_setting);
+	void __iomem *phy_base = phy_ctrl->base;
+	int i;
+
+	fsd_pcie_phy_reset(phy);
+
+	if (phy_ctrl->phy_id == 1) {
+		writel(0x2, phy_base + FSD_PCIE_PHY_CMN_RESET);
+		phy_param = fsd_pcie_phy1_setting;
+		len = ARRAY_SIZE(fsd_pcie_phy1_setting);
+	}
+
+	writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_0);
+	writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_1);
+	writel(0x00, phy_base + FSD_PCIE_PHY_AGG_BIF_RESET);
+	writel(0x00, phy_base + FSD_PCIE_PHY_AGG_BIF_CLOCK);
+
+	for (i = 0; i < len; i++) {
+		if (phy_param[i].is_cmn_reg)
+			writel(phy_param[i].val, phy_base + phy_param[i].addr);
+		else
+			fsd_pcie_phy_writel(phy_ctrl, phy_param[i].val, phy_param[i].addr);
+	}
+
+	if (phy_ctrl->phy_id == 1)
+		writel(0x3, phy_base + FSD_PCIE_PHY_CMN_RESET);
+
+	regmap_update_bits(phy_ctrl->fsysreg, pdata->phy_con,
+			pdata->phy_cmn_rstn_mask, pdata->phy_cmn_rstn);
+
+	return 0;
+}
+
+static const struct phy_ops fsd_phy_ops = {
+	.init		= fsd_pcie_phy_init,
+	.reset		= fsd_pcie_phy_reset,
+	.owner		= THIS_MODULE,
+};
+
+static const struct samsung_drv_data exynos5433_drv_data = {
+	.phy_ops		= &exynos5433_phy_ops,
+};
+
+static const struct samsung_drv_data fsd_drv_data = {
+	.phy_ops		= &fsd_phy_ops,
+};
+
 static const struct of_device_id exynos_pcie_phy_match[] = {
 	{
 		.compatible = "samsung,exynos5433-pcie-phy",
+		.data = &exynos5433_drv_data,
+	},
+	{
+		.compatible = "tesla,fsd-pcie-phy",
+		.data = &fsd_drv_data,
 	},
 	{},
 };
@@ -146,11 +491,18 @@ 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 samsung_drv_data *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)
 		return -ENOMEM;
 
+	exynos_phy->drv_data = drv_data;
+
 	exynos_phy->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(exynos_phy->base))
 		return PTR_ERR(exynos_phy->base);
@@ -169,12 +521,15 @@ 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->phy_ops);
 	if (IS_ERR(generic_phy)) {
 		dev_err(dev, "failed to create PHY\n");
 		return PTR_ERR(generic_phy);
 	}
 
+	exynos_phy->pcs_base = devm_platform_ioremap_resource(pdev, 1);
+	exynos_phy->phy_id = of_alias_get_id(dev->of_node, "pciephy");
+
 	phy_set_drvdata(generic_phy, exynos_phy);
 	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
 
-- 
2.49.0

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

[Krzysztof Kozlowski]

On Mon, May 19, 2025 at 01:01:50AM GMT, Shradha Todi wrote:
> Add PCIe PHY support for Tesla FSD SoC.
> 
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  drivers/phy/samsung/phy-exynos-pcie.c | 357 +++++++++++++++++++++++++-
>  1 file changed, 356 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/samsung/phy-exynos-pcie.c b/drivers/phy/samsung/phy-exynos-pcie.c
> index 53c9230c2907..0e4c00c1121e 100644
> --- a/drivers/phy/samsung/phy-exynos-pcie.c
> +++ b/drivers/phy/samsung/phy-exynos-pcie.c
> @@ -34,11 +34,121 @@
>  /* 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_PHY_LANE_OFFSET	0x400
> +#define FSD_PCIE_PHY_TRSV_REG001_LN_N	0x404
> +#define FSD_PCIE_PHY_TRSV_REG002_LN_N	0x408
> +#define FSD_PCIE_PHY_TRSV_REG005_LN_N	0x414
> +#define FSD_PCIE_PHY_TRSV_REG006_LN_N	0x418
> +#define FSD_PCIE_PHY_TRSV_REG007_LN_N	0x41c
> +#define FSD_PCIE_PHY_TRSV_REG009_LN_N	0x424
> +#define FSD_PCIE_PHY_TRSV_REG00A_LN_N	0x428
> +#define FSD_PCIE_PHY_TRSV_REG00C_LN_N	0x430
> +#define FSD_PCIE_PHY_TRSV_REG012_LN_N	0x448
> +#define FSD_PCIE_PHY_TRSV_REG013_LN_N	0x44c
> +#define FSD_PCIE_PHY_TRSV_REG014_LN_N	0x450
> +#define FSD_PCIE_PHY_TRSV_REG015_LN_N	0x454
> +#define FSD_PCIE_PHY_TRSV_REG016_LN_N	0x458
> +#define FSD_PCIE_PHY_TRSV_REG018_LN_N	0x460
> +#define FSD_PCIE_PHY_TRSV_REG020_LN_N	0x480
> +#define FSD_PCIE_PHY_TRSV_REG026_LN_N	0x498
> +#define FSD_PCIE_PHY_TRSV_REG029_LN_N	0x4a4
> +#define FSD_PCIE_PHY_TRSV_REG031_LN_N	0x4c4
> +#define FSD_PCIE_PHY_TRSV_REG036_LN_N	0x4d8
> +#define FSD_PCIE_PHY_TRSV_REG039_LN_N	0x4e4
> +#define FSD_PCIE_PHY_TRSV_REG03B_LN_N	0x4ec
> +#define FSD_PCIE_PHY_TRSV_REG03C_LN_N	0x4f0
> +#define FSD_PCIE_PHY_TRSV_REG03E_LN_N	0x4f8
> +#define FSD_PCIE_PHY_TRSV_REG03F_LN_N	0x4fc
> +#define FSD_PCIE_PHY_TRSV_REG043_LN_N	0x50c
> +#define FSD_PCIE_PHY_TRSV_REG044_LN_N	0x510
> +#define FSD_PCIE_PHY_TRSV_REG046_LN_N	0x518
> +#define FSD_PCIE_PHY_TRSV_REG048_LN_N	0x520
> +#define FSD_PCIE_PHY_TRSV_REG049_LN_N	0x524
> +#define FSD_PCIE_PHY_TRSV_REG04E_LN_N	0x538
> +#define FSD_PCIE_PHY_TRSV_REG052_LN_N	0x548
> +#define FSD_PCIE_PHY_TRSV_REG068_LN_N	0x5a0
> +#define FSD_PCIE_PHY_TRSV_REG069_LN_N	0x5a4
> +#define FSD_PCIE_PHY_TRSV_REG06A_LN_N	0x5a8
> +#define FSD_PCIE_PHY_TRSV_REG06B_LN_N	0x5ac
> +#define FSD_PCIE_PHY_TRSV_REG07B_LN_N	0x5ec
> +#define FSD_PCIE_PHY_TRSV_REG083_LN_N	0x60c
> +#define FSD_PCIE_PHY_TRSV_REG084_LN_N	0x610
> +#define FSD_PCIE_PHY_TRSV_REG086_LN_N	0x618
> +#define FSD_PCIE_PHY_TRSV_REG087_LN_N	0x61c
> +#define FSD_PCIE_PHY_TRSV_REG08B_LN_N	0x62c
> +#define FSD_PCIE_PHY_TRSV_REG09C_LN_N	0x670
> +#define FSD_PCIE_PHY_TRSV_REG09D_LN_N	0x674
> +#define FSD_PCIE_PHY_TRSV_REG09E_LN_N	0x678
> +#define FSD_PCIE_PHY_TRSV_REG09F_LN_N	0x67c
> +#define FSD_PCIE_PHY_TRSV_REG0A2_LN_N	0x688
> +#define FSD_PCIE_PHY_TRSV_REG0A4_LN_N	0x690
> +#define FSD_PCIE_PHY_TRSV_REG0CE_LN_N	0x738
> +#define FSD_PCIE_PHY_TRSV_REG0FC_LN_N	0x7f0
> +#define FSD_PCIE_PHY_TRSV_REG0FD_LN_N	0x7f4
> +#define FSD_PCIE_PHY_TRSV_REG0FE_LN_N	0x7f8
> +#define FSD_PCIE_PHY_TRSV_REG0CE_LN_1	0xb38
> +#define FSD_PCIE_PHY_TRSV_REG0CE_LN_2	0xf38
> +#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_MASK			0x3ff
> +#define FSD_PCIE_SYSREG_PHY_0_CON			0x042C
> +#define FSD_PCIE_SYSREG_PHY_0_REF_SEL_MASK		0x3
> +#define FSD_PCIE_SYSREG_PHY_0_REF_SEL			(0x2 << 0)
> +#define FSD_PCIE_SYSREG_PHY_0_SSC_EN_MASK		0x8
> +#define FSD_PCIE_SYSREG_PHY_0_SSC_EN			BIT(3)
> +#define FSD_PCIE_SYSREG_PHY_0_AUX_EN_MASK		0x10
> +#define FSD_PCIE_SYSREG_PHY_0_AUX_EN			BIT(4)
> +#define FSD_PCIE_SYSREG_PHY_0_CMN_RSTN_MASK		0x100
> +#define FSD_PCIE_SYSREG_PHY_0_CMN_RSTN			BIT(8)
> +#define FSD_PCIE_SYSREG_PHY_0_INIT_RSTN_MASK		0x200
> +#define FSD_PCIE_SYSREG_PHY_0_INIT_RSTN			BIT(9)
> +
> +#define FSD_PCIE_SYSREG_PHY_1_CON_MASK			0x1ff
> +#define FSD_PCIE_SYSREG_PHY_1_CON			0x0500
> +#define FSD_PCIE_SYSREG_PHY_1_REF_SEL_MASK		0x30
> +#define FSD_PCIE_SYSREG_PHY_1_REF_SEL			(0x2 << 4)
> +#define FSD_PCIE_SYSREG_PHY_1_SSC_EN_MASK		0x80
> +#define FSD_PCIE_SYSREG_PHY_1_SSC_EN			BIT(7)
> +#define FSD_PCIE_SYSREG_PHY_1_AUX_EN_MASK		0x1
> +#define FSD_PCIE_SYSREG_PHY_1_AUX_EN			BIT(0)
> +#define FSD_PCIE_SYSREG_PHY_1_CMN_RSTN_MASK		0x2
> +#define FSD_PCIE_SYSREG_PHY_1_CMN_RSTN			BIT(1)
> +#define FSD_PCIE_SYSREG_PHY_1_INIT_RSTN_MASK		0x8
> +#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;
> +	int phy_id;
> +	const struct samsung_drv_data *drv_data;
> +};
> +
> +struct samsung_drv_data {
> +	const struct phy_ops *phy_ops;
>  };
>  
>  static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
> @@ -133,9 +243,244 @@ static const struct phy_ops exynos5433_phy_ops = {
>  	.owner		= THIS_MODULE,
>  };
>  
> +struct fsd_pcie_phy_pdata {
> +	u32 phy_con_mask;
> +	u32 phy_con;
> +	u32 phy_ref_sel_mask;
> +	u32 phy_ref_sel;
> +	u32 phy_ssc_en_mask;
> +	u32 phy_ssc_en;
> +	u32 phy_aux_en_mask;
> +	u32 phy_aux_en;
> +	u32 phy_cmn_rstn_mask;
> +	u32 phy_cmn_rstn;
> +	u32 phy_init_rstn_mask;
> +	u32 phy_init_rstn;
> +	u32 num_lanes;
> +	u32 lane_offset;
> +};
> +
> +struct fsd_pcie_phy_pdata fsd_phy_con[] = {
> +	{

Why this is global and RW?

> +	.phy_con		= FSD_PCIE_SYSREG_PHY_0_CON,
> +	.phy_con_mask		= FSD_PCIE_SYSREG_PHY_0_CON_MASK,
> +	.phy_ref_sel_mask	= FSD_PCIE_SYSREG_PHY_0_REF_SEL_MASK,
> +	.phy_ref_sel		= FSD_PCIE_SYSREG_PHY_0_REF_SEL,
> +	.phy_ssc_en_mask	= FSD_PCIE_SYSREG_PHY_0_SSC_EN_MASK,
> +	.phy_ssc_en		= FSD_PCIE_SYSREG_PHY_0_SSC_EN,
> +	.phy_aux_en_mask	= FSD_PCIE_SYSREG_PHY_0_AUX_EN_MASK,
> +	.phy_aux_en		= FSD_PCIE_SYSREG_PHY_0_AUX_EN,
> +	.phy_cmn_rstn_mask	= FSD_PCIE_SYSREG_PHY_0_CMN_RSTN_MASK,
> +	.phy_cmn_rstn		= FSD_PCIE_SYSREG_PHY_0_CMN_RSTN,
> +	.phy_init_rstn_mask	= FSD_PCIE_SYSREG_PHY_0_INIT_RSTN_MASK,
> +	.phy_init_rstn		= FSD_PCIE_SYSREG_PHY_0_INIT_RSTN,
> +	.num_lanes		= 0x4,
> +	.lane_offset		= FSD_PCIE_PHY_LANE_OFFSET,
> +	},
> +	{
> +	.phy_con		= FSD_PCIE_SYSREG_PHY_1_CON,
> +	.phy_con_mask		= FSD_PCIE_SYSREG_PHY_1_CON_MASK,
> +	.phy_ref_sel_mask	= FSD_PCIE_SYSREG_PHY_1_REF_SEL_MASK,
> +	.phy_ref_sel		= FSD_PCIE_SYSREG_PHY_1_REF_SEL,
> +	.phy_ssc_en_mask	= FSD_PCIE_SYSREG_PHY_1_SSC_EN_MASK,
> +	.phy_ssc_en		= FSD_PCIE_SYSREG_PHY_1_SSC_EN,
> +	.phy_aux_en_mask	= FSD_PCIE_SYSREG_PHY_1_AUX_EN_MASK,
> +	.phy_aux_en		= FSD_PCIE_SYSREG_PHY_1_AUX_EN,
> +	.phy_cmn_rstn_mask	= FSD_PCIE_SYSREG_PHY_1_CMN_RSTN_MASK,
> +	.phy_cmn_rstn		= FSD_PCIE_SYSREG_PHY_1_CMN_RSTN,
> +	.phy_init_rstn_mask	= FSD_PCIE_SYSREG_PHY_1_INIT_RSTN_MASK,
> +	.phy_init_rstn		= FSD_PCIE_SYSREG_PHY_1_INIT_RSTN,
> +	.num_lanes		= 0x4,
> +	.lane_offset		= FSD_PCIE_PHY_LANE_OFFSET,
> +	},
> +	{ },
> +};
> +
> +struct fsd_pcie_phy_setting {
> +	u32 addr;
> +	u32 val;
> +	bool is_cmn_reg;
> +};
> +
> +struct fsd_pcie_phy_setting fsd_pcie_phy0_setting[] = {

No. This is poor coding, please do first extensive internal reviews.

Please run standard kernel tools for static analysis, like coccinelle,
smatch and sparse, and fix reported warnings. Also please check for
warnings when building with W=1 for gcc and clang. Most of these
commands (checks or W=1 build) can build specific targets, like some
directory, to narrow the scope to only your code. The code here looks
like it needs a fix. Feel free to get in touch if the warning is not
clear.

...

> @@ -146,11 +491,18 @@ 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 samsung_drv_data *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)
>  		return -ENOMEM;
>  
> +	exynos_phy->drv_data = drv_data;
> +
>  	exynos_phy->base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(exynos_phy->base))
>  		return PTR_ERR(exynos_phy->base);
> @@ -169,12 +521,15 @@ 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->phy_ops);
>  	if (IS_ERR(generic_phy)) {
>  		dev_err(dev, "failed to create PHY\n");
>  		return PTR_ERR(generic_phy);
>  	}
>  
> +	exynos_phy->pcs_base = devm_platform_ioremap_resource(pdev, 1);
> +	exynos_phy->phy_id = of_alias_get_id(dev->of_node, "pciephy");

Where did you document aliases?

Anyway, all this looks because you have completely buggy way of handling
MMIO via syscon. That's a no-go. Use proper address ranges assigned to
ddevices. If you ever need to use syscon, you should pass the offset as
argument - just like other devices are doing.

Best regards,
Krzysztof

RE: [PATCH 08/10] phy: exynos: Add PCIe PHY support for FSD SoC

[Shradha Todi]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 21 May 2025 15:11
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.or;
> linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> kw@linux.com; 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
> Subject: Re: [PATCH 08/10] phy: exynos: Add PCIe PHY support for FSD SoC
> 
> On Mon, May 19, 2025 at 01:01:50AM GMT, Shradha Todi wrote:
> > Add PCIe PHY support for Tesla FSD SoC.
> >
> > Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> > ---
> >  drivers/phy/samsung/phy-exynos-pcie.c | 357
> > +++++++++++++++++++++++++-
> >  1 file changed, 356 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/phy/samsung/phy-exynos-pcie.c
> > b/drivers/phy/samsung/phy-exynos-pcie.c
> > index 53c9230c2907..0e4c00c1121e 100644
> > --- a/drivers/phy/samsung/phy-exynos-pcie.c
> > +++ b/drivers/phy/samsung/phy-exynos-pcie.c
> > @@ -34,11 +34,121 @@
> >  /* 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_PHY_LANE_OFFSET	0x400
> > +#define FSD_PCIE_PHY_TRSV_REG001_LN_N	0x404
> > +#define FSD_PCIE_PHY_TRSV_REG002_LN_N	0x408
> > +#define FSD_PCIE_PHY_TRSV_REG005_LN_N	0x414
> > +#define FSD_PCIE_PHY_TRSV_REG006_LN_N	0x418
> > +#define FSD_PCIE_PHY_TRSV_REG007_LN_N	0x41c
> > +#define FSD_PCIE_PHY_TRSV_REG009_LN_N	0x424
> > +#define FSD_PCIE_PHY_TRSV_REG00A_LN_N	0x428
> > +#define FSD_PCIE_PHY_TRSV_REG00C_LN_N	0x430
> > +#define FSD_PCIE_PHY_TRSV_REG012_LN_N	0x448
> > +#define FSD_PCIE_PHY_TRSV_REG013_LN_N	0x44c
> > +#define FSD_PCIE_PHY_TRSV_REG014_LN_N	0x450
> > +#define FSD_PCIE_PHY_TRSV_REG015_LN_N	0x454
> > +#define FSD_PCIE_PHY_TRSV_REG016_LN_N	0x458
> > +#define FSD_PCIE_PHY_TRSV_REG018_LN_N	0x460
> > +#define FSD_PCIE_PHY_TRSV_REG020_LN_N	0x480
> > +#define FSD_PCIE_PHY_TRSV_REG026_LN_N	0x498
> > +#define FSD_PCIE_PHY_TRSV_REG029_LN_N	0x4a4
> > +#define FSD_PCIE_PHY_TRSV_REG031_LN_N	0x4c4
> > +#define FSD_PCIE_PHY_TRSV_REG036_LN_N	0x4d8
> > +#define FSD_PCIE_PHY_TRSV_REG039_LN_N	0x4e4
> > +#define FSD_PCIE_PHY_TRSV_REG03B_LN_N	0x4ec
> > +#define FSD_PCIE_PHY_TRSV_REG03C_LN_N	0x4f0
> > +#define FSD_PCIE_PHY_TRSV_REG03E_LN_N	0x4f8
> > +#define FSD_PCIE_PHY_TRSV_REG03F_LN_N	0x4fc
> > +#define FSD_PCIE_PHY_TRSV_REG043_LN_N	0x50c
> > +#define FSD_PCIE_PHY_TRSV_REG044_LN_N	0x510
> > +#define FSD_PCIE_PHY_TRSV_REG046_LN_N	0x518
> > +#define FSD_PCIE_PHY_TRSV_REG048_LN_N	0x520
> > +#define FSD_PCIE_PHY_TRSV_REG049_LN_N	0x524
> > +#define FSD_PCIE_PHY_TRSV_REG04E_LN_N	0x538
> > +#define FSD_PCIE_PHY_TRSV_REG052_LN_N	0x548
> > +#define FSD_PCIE_PHY_TRSV_REG068_LN_N	0x5a0
> > +#define FSD_PCIE_PHY_TRSV_REG069_LN_N	0x5a4
> > +#define FSD_PCIE_PHY_TRSV_REG06A_LN_N	0x5a8
> > +#define FSD_PCIE_PHY_TRSV_REG06B_LN_N	0x5ac
> > +#define FSD_PCIE_PHY_TRSV_REG07B_LN_N	0x5ec
> > +#define FSD_PCIE_PHY_TRSV_REG083_LN_N	0x60c
> > +#define FSD_PCIE_PHY_TRSV_REG084_LN_N	0x610
> > +#define FSD_PCIE_PHY_TRSV_REG086_LN_N	0x618
> > +#define FSD_PCIE_PHY_TRSV_REG087_LN_N	0x61c
> > +#define FSD_PCIE_PHY_TRSV_REG08B_LN_N	0x62c
> > +#define FSD_PCIE_PHY_TRSV_REG09C_LN_N	0x670
> > +#define FSD_PCIE_PHY_TRSV_REG09D_LN_N	0x674
> > +#define FSD_PCIE_PHY_TRSV_REG09E_LN_N	0x678
> > +#define FSD_PCIE_PHY_TRSV_REG09F_LN_N	0x67c
> > +#define FSD_PCIE_PHY_TRSV_REG0A2_LN_N	0x688
> > +#define FSD_PCIE_PHY_TRSV_REG0A4_LN_N	0x690
> > +#define FSD_PCIE_PHY_TRSV_REG0CE_LN_N	0x738
> > +#define FSD_PCIE_PHY_TRSV_REG0FC_LN_N	0x7f0
> > +#define FSD_PCIE_PHY_TRSV_REG0FD_LN_N	0x7f4
> > +#define FSD_PCIE_PHY_TRSV_REG0FE_LN_N	0x7f8
> > +#define FSD_PCIE_PHY_TRSV_REG0CE_LN_1	0xb38
> > +#define FSD_PCIE_PHY_TRSV_REG0CE_LN_2	0xf38
> > +#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_MASK			0x3ff
> > +#define FSD_PCIE_SYSREG_PHY_0_CON			0x042C
> > +#define FSD_PCIE_SYSREG_PHY_0_REF_SEL_MASK		0x3
> > +#define FSD_PCIE_SYSREG_PHY_0_REF_SEL			(0x2 << 0)
> > +#define FSD_PCIE_SYSREG_PHY_0_SSC_EN_MASK		0x8
> > +#define FSD_PCIE_SYSREG_PHY_0_SSC_EN			BIT(3)
> > +#define FSD_PCIE_SYSREG_PHY_0_AUX_EN_MASK		0x10
> > +#define FSD_PCIE_SYSREG_PHY_0_AUX_EN			BIT(4)
> > +#define FSD_PCIE_SYSREG_PHY_0_CMN_RSTN_MASK		0x100
> > +#define FSD_PCIE_SYSREG_PHY_0_CMN_RSTN			BIT(8)
> > +#define FSD_PCIE_SYSREG_PHY_0_INIT_RSTN_MASK		0x200
> > +#define FSD_PCIE_SYSREG_PHY_0_INIT_RSTN			BIT(9)
> > +
> > +#define FSD_PCIE_SYSREG_PHY_1_CON_MASK			0x1ff
> > +#define FSD_PCIE_SYSREG_PHY_1_CON			0x0500
> > +#define FSD_PCIE_SYSREG_PHY_1_REF_SEL_MASK		0x30
> > +#define FSD_PCIE_SYSREG_PHY_1_REF_SEL			(0x2 << 4)
> > +#define FSD_PCIE_SYSREG_PHY_1_SSC_EN_MASK		0x80
> > +#define FSD_PCIE_SYSREG_PHY_1_SSC_EN			BIT(7)
> > +#define FSD_PCIE_SYSREG_PHY_1_AUX_EN_MASK		0x1
> > +#define FSD_PCIE_SYSREG_PHY_1_AUX_EN			BIT(0)
> > +#define FSD_PCIE_SYSREG_PHY_1_CMN_RSTN_MASK		0x2
> > +#define FSD_PCIE_SYSREG_PHY_1_CMN_RSTN			BIT(1)
> > +#define FSD_PCIE_SYSREG_PHY_1_INIT_RSTN_MASK		0x8
> > +#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;
> > +	int phy_id;
> > +	const struct samsung_drv_data *drv_data; };
> > +
> > +struct samsung_drv_data {
> > +	const struct phy_ops *phy_ops;
> >  };
> >
> >  static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32
> > offset) @@ -133,9 +243,244 @@ static const struct phy_ops exynos5433_phy_ops = {
> >  	.owner		= THIS_MODULE,
> >  };
> >
> > +struct fsd_pcie_phy_pdata {
> > +	u32 phy_con_mask;
> > +	u32 phy_con;
> > +	u32 phy_ref_sel_mask;
> > +	u32 phy_ref_sel;
> > +	u32 phy_ssc_en_mask;
> > +	u32 phy_ssc_en;
> > +	u32 phy_aux_en_mask;
> > +	u32 phy_aux_en;
> > +	u32 phy_cmn_rstn_mask;
> > +	u32 phy_cmn_rstn;
> > +	u32 phy_init_rstn_mask;
> > +	u32 phy_init_rstn;
> > +	u32 num_lanes;
> > +	u32 lane_offset;
> > +};
> > +
> > +struct fsd_pcie_phy_pdata fsd_phy_con[] = {
> > +	{
> 
> Why this is global and RW?
> 
> > +	.phy_con		= FSD_PCIE_SYSREG_PHY_0_CON,
> > +	.phy_con_mask		= FSD_PCIE_SYSREG_PHY_0_CON_MASK,
> > +	.phy_ref_sel_mask	= FSD_PCIE_SYSREG_PHY_0_REF_SEL_MASK,
> > +	.phy_ref_sel		= FSD_PCIE_SYSREG_PHY_0_REF_SEL,
> > +	.phy_ssc_en_mask	= FSD_PCIE_SYSREG_PHY_0_SSC_EN_MASK,
> > +	.phy_ssc_en		= FSD_PCIE_SYSREG_PHY_0_SSC_EN,
> > +	.phy_aux_en_mask	= FSD_PCIE_SYSREG_PHY_0_AUX_EN_MASK,
> > +	.phy_aux_en		= FSD_PCIE_SYSREG_PHY_0_AUX_EN,
> > +	.phy_cmn_rstn_mask	= FSD_PCIE_SYSREG_PHY_0_CMN_RSTN_MASK,
> > +	.phy_cmn_rstn		= FSD_PCIE_SYSREG_PHY_0_CMN_RSTN,
> > +	.phy_init_rstn_mask	= FSD_PCIE_SYSREG_PHY_0_INIT_RSTN_MASK,
> > +	.phy_init_rstn		= FSD_PCIE_SYSREG_PHY_0_INIT_RSTN,
> > +	.num_lanes		= 0x4,
> > +	.lane_offset		= FSD_PCIE_PHY_LANE_OFFSET,
> > +	},
> > +	{
> > +	.phy_con		= FSD_PCIE_SYSREG_PHY_1_CON,
> > +	.phy_con_mask		= FSD_PCIE_SYSREG_PHY_1_CON_MASK,
> > +	.phy_ref_sel_mask	= FSD_PCIE_SYSREG_PHY_1_REF_SEL_MASK,
> > +	.phy_ref_sel		= FSD_PCIE_SYSREG_PHY_1_REF_SEL,
> > +	.phy_ssc_en_mask	= FSD_PCIE_SYSREG_PHY_1_SSC_EN_MASK,
> > +	.phy_ssc_en		= FSD_PCIE_SYSREG_PHY_1_SSC_EN,
> > +	.phy_aux_en_mask	= FSD_PCIE_SYSREG_PHY_1_AUX_EN_MASK,
> > +	.phy_aux_en		= FSD_PCIE_SYSREG_PHY_1_AUX_EN,
> > +	.phy_cmn_rstn_mask	= FSD_PCIE_SYSREG_PHY_1_CMN_RSTN_MASK,
> > +	.phy_cmn_rstn		= FSD_PCIE_SYSREG_PHY_1_CMN_RSTN,
> > +	.phy_init_rstn_mask	= FSD_PCIE_SYSREG_PHY_1_INIT_RSTN_MASK,
> > +	.phy_init_rstn		= FSD_PCIE_SYSREG_PHY_1_INIT_RSTN,
> > +	.num_lanes		= 0x4,
> > +	.lane_offset		= FSD_PCIE_PHY_LANE_OFFSET,
> > +	},
> > +	{ },
> > +};
> > +
> > +struct fsd_pcie_phy_setting {
> > +	u32 addr;
> > +	u32 val;
> > +	bool is_cmn_reg;
> > +};
> > +
> > +struct fsd_pcie_phy_setting fsd_pcie_phy0_setting[] = {
> 
> No. This is poor coding, please do first extensive internal reviews.
> 
> Please run standard kernel tools for static analysis, like coccinelle, smatch and sparse, and fix reported warnings. Also please check for
> warnings when building with W=1 for gcc and clang. Most of these commands (checks or W=1 build) can build specific targets, like
> some directory, to narrow the scope to only your code. The code here looks like it needs a fix. Feel free to get in touch if the warning
> is not clear.
> 

Will run all these tools and fix where required

> ...
> 
> > @@ -146,11 +491,18 @@ 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 samsung_drv_data *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)
> >  		return -ENOMEM;
> >
> > +	exynos_phy->drv_data = drv_data;
> > +
> >  	exynos_phy->base = devm_platform_ioremap_resource(pdev, 0);
> >  	if (IS_ERR(exynos_phy->base))
> >  		return PTR_ERR(exynos_phy->base);
> > @@ -169,12 +521,15 @@ 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->phy_ops);
> >  	if (IS_ERR(generic_phy)) {
> >  		dev_err(dev, "failed to create PHY\n");
> >  		return PTR_ERR(generic_phy);
> >  	}
> >
> > +	exynos_phy->pcs_base = devm_platform_ioremap_resource(pdev, 1);
> > +	exynos_phy->phy_id = of_alias_get_id(dev->of_node, "pciephy");
> 
> Where did you document aliases?
> 

Will add it to dt bindings.

> Anyway, all this looks because you have completely buggy way of handling MMIO via syscon. That's a no-go. Use proper address
> ranges assigned to ddevices. If you ever need to use syscon, you should pass the offset as argument - just like other devices are
> doing.
> 

Alias is used for 2 reasons.
1. Each of the 2 PHYs in FSD have different initializing sequence due to channel length, etc. We need the alias to select the init sequence accordingly
2. The syscon offset can be passed via DT but the bit field also varies according to instance. (common reset is bit 8 in PHY0 and bit 1 in PHY1).

I could read the sysreg offset from DT and based on the offset value, decide which instance it is. Would that be okay?

> Best regards,
> Krzysztof

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

[Krzysztof Kozlowski]

On 27/05/2025 12:45, Shradha Todi wrote:
>>>
>>> -	generic_phy = devm_phy_create(dev, dev->of_node, &exynos5433_phy_ops);
>>> +	generic_phy = devm_phy_create(dev, dev->of_node, drv_data->phy_ops);
>>>  	if (IS_ERR(generic_phy)) {
>>>  		dev_err(dev, "failed to create PHY\n");
>>>  		return PTR_ERR(generic_phy);
>>>  	}
>>>
>>> +	exynos_phy->pcs_base = devm_platform_ioremap_resource(pdev, 1);
>>> +	exynos_phy->phy_id = of_alias_get_id(dev->of_node, "pciephy");
>>
>> Where did you document aliases?
>>
> 
> Will add it to dt bindings.
> 
>> Anyway, all this looks because you have completely buggy way of handling MMIO via syscon. That's a no-go. Use proper address
>> ranges assigned to ddevices. If you ever need to use syscon, you should pass the offset as argument - just like other devices are
>> doing.
>>
> 
> Alias is used for 2 reasons.

So if on my board the PCI slots are named differently and I use
different alias, everything will stop working, right? Usually aliases
for exposable buses are matching what is physically labeled on the
exposed interface.

> 1. Each of the 2 PHYs in FSD have different initializing sequence due to channel length, etc. We need the alias to select the init sequence accordingly

So devices are different? What is channel length? Number of lanes?


> 2. The syscon offset can be passed via DT but the bit field also varies according to instance. (common reset is bit 8 in PHY0 and bit 1 in PHY1).

You did not address the main problem here: you use MMIO but do not
define any MMIO. Syscon is not a replacement for MMIO.


Best regards,
Krzysztof

[PATCH 09/10] 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 | 330 +++++++++++++++++++++++-
 1 file changed, 322 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index b122a2ae8681..97953cc73aa2 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -20,6 +20,8 @@
 #include <linux/regulator/consumer.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
 
 #include "pcie-designware.h"
 
@@ -49,17 +51,46 @@
 #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;
 };
 
 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;
@@ -69,6 +100,7 @@ struct exynos_pcie {
 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)
@@ -328,11 +360,202 @@ 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 int 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:
+	case PCI_IRQ_MSIX:
+		dev_err(pci->dev, "EP does not support legacy IRQs\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;
@@ -355,6 +578,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))
@@ -375,13 +618,43 @@ static int exynos_pcie_probe(struct platform_device *pdev)
 		return ret;
 
 	platform_set_drvdata(pdev, ep);
-	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)
+
+	if (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;
+	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");
 		goto fail_phy_init;
+	}
 
 	return 0;
 
@@ -397,8 +670,11 @@ 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);
 	samsung_regulator_disable(ep);
@@ -407,8 +683,16 @@ static void exynos_pcie_remove(struct platform_device *pdev)
 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->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);
 	phy_exit(ep->phy);
 	samsung_regulator_disable(ep);
@@ -423,6 +707,9 @@ static int exynos_pcie_resume_noirq(struct device *dev)
 	struct dw_pcie_rp *pp = &pci->pp;
 	int ret;
 
+	if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
+		return 0;
+
 	ret = samsung_regulator_enable(ep);
 	if (ret)
 		return ret;
@@ -439,11 +726,30 @@ 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,
 	.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[] = {
@@ -451,6 +757,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 09/10] PCI: exynos: Add support for Tesla FSD SoC

[Niklas Cassel]

On Mon, May 19, 2025 at 01:01:51AM +0530, Shradha Todi wrote:
> Add host and endpoint controller driver support for FSD SoC.
> 
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---

(snip)

> +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:
> +	case PCI_IRQ_MSIX:
> +		dev_err(pci->dev, "EP does not support legacy IRQs\n");

Here you will print the same message for both INTX and MSIX.
Perhaps MSIX should have a separate print?

In fact, perhaps you want to call dw_pcie_ep_raise_intx_irq()
for case PCI_IRQ_INTX, since that function already has an error print.


Kind regards,
Niklas

Re: [PATCH 09/10] PCI: exynos: Add support for Tesla FSD SoC

[Krzysztof Kozlowski]

On Mon, May 19, 2025 at 01:01:51AM GMT, Shradha Todi wrote:
>  static int exynos_pcie_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -355,6 +578,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");

So all MMIO will go via syscon? I am pretty close to NAKing all this,
but let's be sure that I got it right - please post your complete DTS
for upstream. That's a requirement from me for any samsung drivers - I
don't want to support fake, broken downstream solutions (based on
multiple past submissions).

Best regards,
Krzysztof

RE: [PATCH 09/10] PCI: exynos: Add support for Tesla FSD SoC

[Shradha Todi]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 21 May 2025 15:18
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.or;
> linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> kw@linux.com; 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
> Subject: Re: [PATCH 09/10] PCI: exynos: Add support for Tesla FSD SoC
> 
> On Mon, May 19, 2025 at 01:01:51AM GMT, Shradha Todi wrote:
> >  static int exynos_pcie_probe(struct platform_device *pdev)  {
> >  	struct device *dev = &pdev->dev;
> > @@ -355,6 +578,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");
> 
> So all MMIO will go via syscon? I am pretty close to NAKing all this, but let's be sure that I got it right - please post your complete DTS
> for upstream. That's a requirement from me for any samsung drivers - I don't want to support fake, broken downstream solutions
> (based on multiple past submissions).
> 

By all MMIO do you mean DBI read/write? The FSD hardware architecture is such that the DBI/ATU/DMA address is at the same offset.
The syscon register holds the upper bits of the actual address differentiating between these 3 spaces. This kind of implementation was done
to reduce address space for PCI DWC controller. So yes, each DBI/ATU register read/write will have syscon write before it to switch address space.

> Best regards,
> Krzysztof

Re: [PATCH 09/10] PCI: exynos: Add support for Tesla FSD SoC

[Krzysztof Kozlowski]

On 27/05/2025 12:45, Shradha Todi wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: 21 May 2025 15:18
>> To: Shradha Todi <shradha.t@samsung.com>
>> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.or;
>> linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
>> kw@linux.com; 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
>> Subject: Re: [PATCH 09/10] PCI: exynos: Add support for Tesla FSD SoC
>>
>> On Mon, May 19, 2025 at 01:01:51AM GMT, Shradha Todi wrote:
>>>  static int exynos_pcie_probe(struct platform_device *pdev)  {
>>>  	struct device *dev = &pdev->dev;
>>> @@ -355,6 +578,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");
>>
>> So all MMIO will go via syscon? I am pretty close to NAKing all this, but let's be sure that I got it right - please post your complete DTS
>> for upstream. That's a requirement from me for any samsung drivers - I don't want to support fake, broken downstream solutions
>> (based on multiple past submissions).
>>
> 
> By all MMIO do you mean DBI read/write? The FSD hardware architecture is such that the DBI/ATU/DMA address is at the same offset.
> The syscon register holds the upper bits of the actual address differentiating between these 3 spaces. This kind of implementation was done
> to reduce address space for PCI DWC controller. So yes, each DBI/ATU register read/write will have syscon write before it to switch address space.

Wrap your replies correctly to fit mailing list.

No, I meant your binding does not define any MMIO at all. I see you use
for example elbi_base which is mapped from "elbi" reg entry, but you do
not have it in your binding.

Maybe just binding is heavily incomplete and that confused me.

Best regards,
Krzysztof

RE: [PATCH 09/10] PCI: exynos: Add support for Tesla FSD SoC

[Shradha Todi]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 28 May 2025 12:56
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-samsung-soc@vger.kernel.or; linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org;
> manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org; kw@linux.com; 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
> Subject: Re: [PATCH 09/10] PCI: exynos: Add support for Tesla FSD SoC
> 
> On 27/05/2025 12:45, Shradha Todi wrote:
> >
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: 21 May 2025 15:18
> >> To: Shradha Todi <shradha.t@samsung.com>
> >> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-samsung-soc@vger.kernel.or;
> >> linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org; manivannan.sadhasivam@linaro.org;
> lpieralisi@kernel.org;
> >> kw@linux.com; 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
> >> Subject: Re: [PATCH 09/10] PCI: exynos: Add support for Tesla FSD SoC
> >>
> >> On Mon, May 19, 2025 at 01:01:51AM GMT, Shradha Todi wrote:
> >>>  static int exynos_pcie_probe(struct platform_device *pdev)  {
> >>>  	struct device *dev = &pdev->dev;
> >>> @@ -355,6 +578,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");
> >>
> >> So all MMIO will go via syscon? I am pretty close to NAKing all this, but let's be sure that I got it right
> - please post your complete DTS
> >> for upstream. That's a requirement from me for any samsung drivers - I don't want to support fake,
> broken downstream solutions
> >> (based on multiple past submissions).
> >>
> >
> > By all MMIO do you mean DBI read/write? The FSD hardware architecture is such that the
> > DBI/ATU/DMA address is at the same offset.
> > The syscon register holds the upper bits of the actual address differentiating between these 3
> > spaces. This kind of implementation was done
> > to reduce address space for PCI DWC controller. So yes, each DBI/ATU register read/write will have
> > syscon write before it to switch address space.
> 
> Wrap your replies correctly to fit mailing list.
> 
> No, I meant your binding does not define any MMIO at all. I see you use
> for example elbi_base which is mapped from "elbi" reg entry, but you do
> not have it in your binding.
> 
> Maybe just binding is heavily incomplete and that confused me.
> 

Got it. I think the confusion is due to the incomplete dt-bindings.
I will fix these issues in the next version. Will post again once I get
clarity about how to avoid redirection in patch 4/10

> Best regards,
> Krzysztof

[PATCH 10/10] misc: pci_endpoint_test: Add driver data for FSD PCIe controllers

[Shradha Todi]

dma_map_single() might not return a 4KB aligned address, so add the
default_data as driver data for FSD PCIe controllers to make it
4KB aligned.

Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 drivers/misc/pci_endpoint_test.c | 3 +++
 include/linux/pci_ids.h          | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index c4e5e2c977be..d94a94231ee5 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -1110,6 +1110,9 @@ static const struct pci_device_id pci_endpoint_test_tbl[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, PCI_DEVICE_ID_LS1088A),
 	  .driver_data = (kernel_ulong_t)&default_data,
 	},
+	{ PCI_DEVICE(PCI_VENDOR_ID_TESLA, 0x7777),
+	  .driver_data = (kernel_ulong_t)&default_data,
+	},
 	{ PCI_DEVICE_DATA(SYNOPSYS, EDDA, NULL) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_DEVICE_ID_TI_AM654),
 	  .driver_data = (kernel_ulong_t)&am654_data
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 2e28182c3af0..e0afc5aa1c0e 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -167,6 +167,8 @@
 
 #define PCI_VENDOR_ID_SOLIDIGM		0x025e
 
+#define PCI_VENDOR_ID_TESLA		0x014a
+
 #define PCI_VENDOR_ID_TTTECH		0x0357
 #define PCI_DEVICE_ID_TTTECH_MC322	0x000a
 
-- 
2.49.0

Re: [PATCH 10/10] misc: pci_endpoint_test: Add driver data for FSD PCIe controllers

[Niklas Cassel]

Hello Shradha,

On Mon, May 19, 2025 at 01:01:52AM +0530, Shradha Todi wrote:
> dma_map_single() might not return a 4KB aligned address, so add the
> default_data as driver data for FSD PCIe controllers to make it
> 4KB aligned.
> 
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  drivers/misc/pci_endpoint_test.c | 3 +++
>  include/linux/pci_ids.h          | 2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index c4e5e2c977be..d94a94231ee5 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -1110,6 +1110,9 @@ static const struct pci_device_id pci_endpoint_test_tbl[] = {
>  	{ PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, PCI_DEVICE_ID_LS1088A),
>  	  .driver_data = (kernel_ulong_t)&default_data,
>  	},
> +	{ PCI_DEVICE(PCI_VENDOR_ID_TESLA, 0x7777),
> +	  .driver_data = (kernel_ulong_t)&default_data,
> +	},

Are you sure that you actually require this?

Since we now have these two commits:
e73ea1c2d4d8 ("PCI: dwc: endpoint: Implement the pci_epc_ops::align_addr() operation")
0d292a1e6d90 ("misc: pci_endpoint_test: Add support for capabilities")

My expectation is that DWC based endpoint controller drivers should no longer
need an explicit 4k alignment in the host side driver.

Thus, I would expect that:
{ PCI_DEVICE(PCI_VENDOR_ID_TESLA, 0x7777),},

(i.e. no explicit 4k alignment)
should be sufficient.


Kind regards,
Niklas