* [PATCH v3 01/12] PCI: exynos: Remove unused MACROs in exynos PCIe file
2025-08-11 15:46 ` [PATCH v3 00/12] Add PCIe support for Tesla FSD SoC Shradha Todi
@ 2025-08-11 15:46 ` Shradha Todi
2025-08-11 15:46 ` [PATCH v3 02/12] PCI: exynos: Change macro names to exynos specific Shradha Todi
` (10 subsequent siblings)
11 siblings, 0 replies; 34+ messages in thread
From: Shradha Todi @ 2025-08-11 15:46 UTC (permalink / raw)
To: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey, Shradha Todi, Hrishikesh Dileep
Some MACROs are defined in the exynos PCIe file but are
not used anywhere within the file. Remove such unused
MACROs.
Suggested-by: Hrishikesh Dileep <hrishikesh.d@samsung.com>
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
drivers/pci/controller/dwc/pci-exynos.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index 1f0e98d07109..f9140d1f1d19 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -31,8 +31,6 @@
#define IRQ_INTB_ASSERT BIT(2)
#define IRQ_INTC_ASSERT BIT(4)
#define IRQ_INTD_ASSERT BIT(6)
-#define PCIE_IRQ_LEVEL 0x004
-#define PCIE_IRQ_SPECIAL 0x008
#define PCIE_IRQ_EN_PULSE 0x00c
#define PCIE_IRQ_EN_LEVEL 0x010
#define PCIE_IRQ_EN_SPECIAL 0x014
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v3 02/12] PCI: exynos: Change macro names to exynos specific
2025-08-11 15:46 ` [PATCH v3 00/12] Add PCIe support for Tesla FSD SoC Shradha Todi
2025-08-11 15:46 ` [PATCH v3 01/12] PCI: exynos: Remove unused MACROs in exynos PCIe file Shradha Todi
@ 2025-08-11 15:46 ` Shradha Todi
2025-08-11 15:46 ` [PATCH v3 03/12] PCI: exynos: Reorder MACROs to maintain consistency Shradha Todi
` (9 subsequent siblings)
11 siblings, 0 replies; 34+ messages in thread
From: Shradha Todi @ 2025-08-11 15:46 UTC (permalink / raw)
To: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey, Shradha Todi
Prefix macro names in exynos file with the term "EXYNOS" as the current
macro names seem to be generic to PCIe.
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
drivers/pci/controller/dwc/pci-exynos.c | 112 ++++++++++++------------
1 file changed, 56 insertions(+), 56 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index f9140d1f1d19..30d12ff9b0c6 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -26,28 +26,28 @@
#define to_exynos_pcie(x) dev_get_drvdata((x)->dev)
/* PCIe ELBI registers */
-#define PCIE_IRQ_PULSE 0x000
-#define IRQ_INTA_ASSERT BIT(0)
-#define IRQ_INTB_ASSERT BIT(2)
-#define IRQ_INTC_ASSERT BIT(4)
-#define IRQ_INTD_ASSERT BIT(6)
-#define PCIE_IRQ_EN_PULSE 0x00c
-#define PCIE_IRQ_EN_LEVEL 0x010
-#define PCIE_IRQ_EN_SPECIAL 0x014
-#define PCIE_SW_WAKE 0x018
-#define PCIE_BUS_EN BIT(1)
-#define PCIE_CORE_RESET 0x01c
-#define PCIE_CORE_RESET_ENABLE BIT(0)
-#define PCIE_STICKY_RESET 0x020
-#define PCIE_NONSTICKY_RESET 0x024
-#define PCIE_APP_INIT_RESET 0x028
-#define PCIE_APP_LTSSM_ENABLE 0x02c
-#define PCIE_ELBI_RDLH_LINKUP 0x074
-#define PCIE_ELBI_XMLH_LINKUP BIT(4)
-#define PCIE_ELBI_LTSSM_ENABLE 0x1
-#define PCIE_ELBI_SLV_AWMISC 0x11c
-#define PCIE_ELBI_SLV_ARMISC 0x120
-#define PCIE_ELBI_SLV_DBI_ENABLE BIT(21)
+#define EXYNOS_PCIE_IRQ_PULSE 0x000
+#define EXYNOS_IRQ_INTA_ASSERT BIT(0)
+#define EXYNOS_IRQ_INTB_ASSERT BIT(2)
+#define EXYNOS_IRQ_INTC_ASSERT BIT(4)
+#define EXYNOS_IRQ_INTD_ASSERT BIT(6)
+#define EXYNOS_PCIE_IRQ_EN_PULSE 0x00c
+#define EXYNOS_PCIE_IRQ_EN_LEVEL 0x010
+#define EXYNOS_PCIE_IRQ_EN_SPECIAL 0x014
+#define EXYNOS_PCIE_SW_WAKE 0x018
+#define EXYNOS_PCIE_BUS_EN BIT(1)
+#define EXYNOS_PCIE_CORE_RESET 0x01c
+#define EXYNOS_PCIE_CORE_RESET_ENABLE BIT(0)
+#define EXYNOS_PCIE_STICKY_RESET 0x020
+#define EXYNOS_PCIE_NONSTICKY_RESET 0x024
+#define EXYNOS_PCIE_APP_INIT_RESET 0x028
+#define EXYNOS_PCIE_APP_LTSSM_ENABLE 0x02c
+#define EXYNOS_PCIE_ELBI_RDLH_LINKUP 0x074
+#define EXYNOS_PCIE_ELBI_XMLH_LINKUP BIT(4)
+#define EXYNOS_PCIE_ELBI_LTSSM_ENABLE 0x1
+#define EXYNOS_PCIE_ELBI_SLV_AWMISC 0x11c
+#define EXYNOS_PCIE_ELBI_SLV_ARMISC 0x120
+#define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE BIT(21)
struct exynos_pcie {
struct dw_pcie pci;
@@ -71,49 +71,49 @@ static void exynos_pcie_sideband_dbi_w_mode(struct exynos_pcie *ep, bool on)
{
u32 val;
- val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_SLV_AWMISC);
+ val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_ELBI_SLV_AWMISC);
if (on)
- val |= PCIE_ELBI_SLV_DBI_ENABLE;
+ val |= EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE;
else
- val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
- exynos_pcie_writel(ep->elbi_base, val, PCIE_ELBI_SLV_AWMISC);
+ val &= ~EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE;
+ exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_ELBI_SLV_AWMISC);
}
static void exynos_pcie_sideband_dbi_r_mode(struct exynos_pcie *ep, bool on)
{
u32 val;
- val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_SLV_ARMISC);
+ val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_ELBI_SLV_ARMISC);
if (on)
- val |= PCIE_ELBI_SLV_DBI_ENABLE;
+ val |= EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE;
else
- val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
- exynos_pcie_writel(ep->elbi_base, val, PCIE_ELBI_SLV_ARMISC);
+ val &= ~EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE;
+ exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_ELBI_SLV_ARMISC);
}
static void exynos_pcie_assert_core_reset(struct exynos_pcie *ep)
{
u32 val;
- val = exynos_pcie_readl(ep->elbi_base, PCIE_CORE_RESET);
- val &= ~PCIE_CORE_RESET_ENABLE;
- exynos_pcie_writel(ep->elbi_base, val, PCIE_CORE_RESET);
- exynos_pcie_writel(ep->elbi_base, 0, PCIE_STICKY_RESET);
- exynos_pcie_writel(ep->elbi_base, 0, PCIE_NONSTICKY_RESET);
+ val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_CORE_RESET);
+ val &= ~EXYNOS_PCIE_CORE_RESET_ENABLE;
+ exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_CORE_RESET);
+ exynos_pcie_writel(ep->elbi_base, 0, EXYNOS_PCIE_STICKY_RESET);
+ exynos_pcie_writel(ep->elbi_base, 0, EXYNOS_PCIE_NONSTICKY_RESET);
}
static void exynos_pcie_deassert_core_reset(struct exynos_pcie *ep)
{
u32 val;
- val = exynos_pcie_readl(ep->elbi_base, PCIE_CORE_RESET);
- val |= PCIE_CORE_RESET_ENABLE;
+ val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_CORE_RESET);
+ val |= EXYNOS_PCIE_CORE_RESET_ENABLE;
- exynos_pcie_writel(ep->elbi_base, val, PCIE_CORE_RESET);
- exynos_pcie_writel(ep->elbi_base, 1, PCIE_STICKY_RESET);
- exynos_pcie_writel(ep->elbi_base, 1, PCIE_NONSTICKY_RESET);
- exynos_pcie_writel(ep->elbi_base, 1, PCIE_APP_INIT_RESET);
- exynos_pcie_writel(ep->elbi_base, 0, PCIE_APP_INIT_RESET);
+ exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_CORE_RESET);
+ exynos_pcie_writel(ep->elbi_base, 1, EXYNOS_PCIE_STICKY_RESET);
+ exynos_pcie_writel(ep->elbi_base, 1, EXYNOS_PCIE_NONSTICKY_RESET);
+ exynos_pcie_writel(ep->elbi_base, 1, EXYNOS_PCIE_APP_INIT_RESET);
+ exynos_pcie_writel(ep->elbi_base, 0, EXYNOS_PCIE_APP_INIT_RESET);
}
static int exynos_pcie_start_link(struct dw_pcie *pci)
@@ -121,21 +121,21 @@ static int exynos_pcie_start_link(struct dw_pcie *pci)
struct exynos_pcie *ep = to_exynos_pcie(pci);
u32 val;
- val = exynos_pcie_readl(ep->elbi_base, PCIE_SW_WAKE);
- val &= ~PCIE_BUS_EN;
- exynos_pcie_writel(ep->elbi_base, val, PCIE_SW_WAKE);
+ val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_SW_WAKE);
+ val &= ~EXYNOS_PCIE_BUS_EN;
+ exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_SW_WAKE);
/* assert LTSSM enable */
- exynos_pcie_writel(ep->elbi_base, PCIE_ELBI_LTSSM_ENABLE,
- PCIE_APP_LTSSM_ENABLE);
+ exynos_pcie_writel(ep->elbi_base, EXYNOS_PCIE_ELBI_LTSSM_ENABLE,
+ EXYNOS_PCIE_APP_LTSSM_ENABLE);
return 0;
}
static void exynos_pcie_clear_irq_pulse(struct exynos_pcie *ep)
{
- u32 val = exynos_pcie_readl(ep->elbi_base, PCIE_IRQ_PULSE);
+ u32 val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_IRQ_PULSE);
- exynos_pcie_writel(ep->elbi_base, val, PCIE_IRQ_PULSE);
+ exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_IRQ_PULSE);
}
static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
@@ -148,12 +148,12 @@ static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
{
- u32 val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
- IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
+ u32 val = EXYNOS_IRQ_INTA_ASSERT | EXYNOS_IRQ_INTB_ASSERT |
+ EXYNOS_IRQ_INTC_ASSERT | EXYNOS_IRQ_INTD_ASSERT;
- exynos_pcie_writel(ep->elbi_base, val, PCIE_IRQ_EN_PULSE);
- exynos_pcie_writel(ep->elbi_base, 0, PCIE_IRQ_EN_LEVEL);
- exynos_pcie_writel(ep->elbi_base, 0, PCIE_IRQ_EN_SPECIAL);
+ exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_IRQ_EN_PULSE);
+ exynos_pcie_writel(ep->elbi_base, 0, EXYNOS_PCIE_IRQ_EN_LEVEL);
+ exynos_pcie_writel(ep->elbi_base, 0, EXYNOS_PCIE_IRQ_EN_SPECIAL);
}
static u32 exynos_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
@@ -210,9 +210,9 @@ static struct pci_ops exynos_pci_ops = {
static bool exynos_pcie_link_up(struct dw_pcie *pci)
{
struct exynos_pcie *ep = to_exynos_pcie(pci);
- u32 val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_RDLH_LINKUP);
+ u32 val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_ELBI_RDLH_LINKUP);
- return val & PCIE_ELBI_XMLH_LINKUP;
+ return val & EXYNOS_PCIE_ELBI_XMLH_LINKUP;
}
static int exynos_pcie_host_init(struct dw_pcie_rp *pp)
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v3 03/12] PCI: exynos: Reorder MACROs to maintain consistency
2025-08-11 15:46 ` [PATCH v3 00/12] Add PCIe support for Tesla FSD SoC Shradha Todi
2025-08-11 15:46 ` [PATCH v3 01/12] PCI: exynos: Remove unused MACROs in exynos PCIe file Shradha Todi
2025-08-11 15:46 ` [PATCH v3 02/12] PCI: exynos: Change macro names to exynos specific Shradha Todi
@ 2025-08-11 15:46 ` Shradha Todi
2025-08-11 15:46 ` [PATCH v3 04/12] PCI: exynos: Add platform device private data Shradha Todi
` (8 subsequent siblings)
11 siblings, 0 replies; 34+ messages in thread
From: Shradha Todi @ 2025-08-11 15:46 UTC (permalink / raw)
To: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey, Shradha Todi, Hrishikesh Dileep
Exynos PCI file follows MACRO definition order where
register offset is defined in ascending order and each
bit field within the offset is defined right after offset
definition. Some MACROs are out of order and so reorder
those MACROs to maintain consistency.
Suggested-by: Hrishikesh Dileep <hrishikesh.d@samsung.com>
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
drivers/pci/controller/dwc/pci-exynos.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index 30d12ff9b0c6..b4ec167b0583 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -27,11 +27,11 @@
/* PCIe ELBI registers */
#define EXYNOS_PCIE_IRQ_PULSE 0x000
+#define EXYNOS_PCIE_IRQ_EN_PULSE 0x00c
#define EXYNOS_IRQ_INTA_ASSERT BIT(0)
#define EXYNOS_IRQ_INTB_ASSERT BIT(2)
#define EXYNOS_IRQ_INTC_ASSERT BIT(4)
#define EXYNOS_IRQ_INTD_ASSERT BIT(6)
-#define EXYNOS_PCIE_IRQ_EN_PULSE 0x00c
#define EXYNOS_PCIE_IRQ_EN_LEVEL 0x010
#define EXYNOS_PCIE_IRQ_EN_SPECIAL 0x014
#define EXYNOS_PCIE_SW_WAKE 0x018
@@ -42,9 +42,9 @@
#define EXYNOS_PCIE_NONSTICKY_RESET 0x024
#define EXYNOS_PCIE_APP_INIT_RESET 0x028
#define EXYNOS_PCIE_APP_LTSSM_ENABLE 0x02c
+#define EXYNOS_PCIE_ELBI_LTSSM_ENABLE 0x1
#define EXYNOS_PCIE_ELBI_RDLH_LINKUP 0x074
#define EXYNOS_PCIE_ELBI_XMLH_LINKUP BIT(4)
-#define EXYNOS_PCIE_ELBI_LTSSM_ENABLE 0x1
#define EXYNOS_PCIE_ELBI_SLV_AWMISC 0x11c
#define EXYNOS_PCIE_ELBI_SLV_ARMISC 0x120
#define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE BIT(21)
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v3 04/12] PCI: exynos: Add platform device private data
2025-08-11 15:46 ` [PATCH v3 00/12] Add PCIe support for Tesla FSD SoC Shradha Todi
` (2 preceding siblings ...)
2025-08-11 15:46 ` [PATCH v3 03/12] PCI: exynos: Reorder MACROs to maintain consistency Shradha Todi
@ 2025-08-11 15:46 ` Shradha Todi
2025-08-11 15:46 ` [PATCH v3 05/12] PCI: exynos: Add resource ops, soc variant and device mode Shradha Todi
` (7 subsequent siblings)
11 siblings, 0 replies; 34+ messages in thread
From: Shradha Todi @ 2025-08-11 15:46 UTC (permalink / raw)
To: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey, Shradha Todi
In order to extend this driver to all Samsung manufactured SoCs having
DWC PCIe controller, add private data structure which will hold platform
device specific information. It holds function ops like DWC host ops,
DWC generic ops, and PCI read/write ops which will be used as driver
data for different compatibles.
Suggested-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
drivers/pci/controller/dwc/pci-exynos.c | 32 ++++++++++++++++++++-----
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index b4ec167b0583..c830b20d54f0 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -49,9 +49,16 @@
#define EXYNOS_PCIE_ELBI_SLV_ARMISC 0x120
#define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE BIT(21)
+struct samsung_pcie_pdata {
+ struct pci_ops *pci_ops;
+ const struct dw_pcie_ops *dwc_ops;
+ const struct dw_pcie_host_ops *host_ops;
+};
+
struct exynos_pcie {
struct dw_pcie pci;
void __iomem *elbi_base;
+ const struct samsung_pcie_pdata *pdata;
struct clk_bulk_data *clks;
struct phy *phy;
struct regulator_bulk_data supplies[2];
@@ -220,7 +227,7 @@ static int exynos_pcie_host_init(struct dw_pcie_rp *pp)
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct exynos_pcie *ep = to_exynos_pcie(pci);
- pp->bridge->ops = &exynos_pci_ops;
+ pp->bridge->ops = ep->pdata->pci_ops;
exynos_pcie_assert_core_reset(ep);
@@ -268,7 +275,7 @@ static int exynos_add_pcie_port(struct exynos_pcie *ep,
return 0;
}
-static const struct dw_pcie_ops dw_pcie_ops = {
+static const struct dw_pcie_ops exynos_dw_pcie_ops = {
.read_dbi = exynos_pcie_read_dbi,
.write_dbi = exynos_pcie_write_dbi,
.link_up = exynos_pcie_link_up,
@@ -279,6 +286,7 @@ static int exynos_pcie_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct exynos_pcie *ep;
+ const struct samsung_pcie_pdata *pdata;
struct device_node *np = dev->of_node;
int ret;
@@ -286,8 +294,11 @@ static int exynos_pcie_probe(struct platform_device *pdev)
if (!ep)
return -ENOMEM;
+ pdata = of_device_get_match_data(dev);
+
+ ep->pdata = pdata;
ep->pci.dev = dev;
- ep->pci.ops = &dw_pcie_ops;
+ ep->pci.ops = pdata->dwc_ops;
ep->phy = devm_of_phy_get(dev, np, NULL);
if (IS_ERR(ep->phy))
@@ -363,9 +374,9 @@ static int exynos_pcie_resume_noirq(struct device *dev)
return ret;
/* exynos_pcie_host_init controls ep->phy */
- exynos_pcie_host_init(pp);
+ ep->pdata->host_ops->init(pp);
dw_pcie_setup_rc(pp);
- exynos_pcie_start_link(pci);
+ ep->pdata->dwc_ops->start_link(pci);
return dw_pcie_wait_for_link(pci);
}
@@ -374,8 +385,17 @@ static const struct dev_pm_ops exynos_pcie_pm_ops = {
exynos_pcie_resume_noirq)
};
+static const struct samsung_pcie_pdata exynos_5433_pcie_rc_pdata = {
+ .dwc_ops = &exynos_dw_pcie_ops,
+ .pci_ops = &exynos_pci_ops,
+ .host_ops = &exynos_pcie_host_ops,
+};
+
static const struct of_device_id exynos_pcie_of_match[] = {
- { .compatible = "samsung,exynos5433-pcie", },
+ {
+ .compatible = "samsung,exynos5433-pcie",
+ .data = (void *) &exynos_5433_pcie_rc_pdata,
+ },
{ },
};
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v3 05/12] PCI: exynos: Add resource ops, soc variant and device mode
2025-08-11 15:46 ` [PATCH v3 00/12] Add PCIe support for Tesla FSD SoC Shradha Todi
` (3 preceding siblings ...)
2025-08-11 15:46 ` [PATCH v3 04/12] PCI: exynos: Add platform device private data Shradha Todi
@ 2025-08-11 15:46 ` Shradha Todi
2025-08-13 23:07 ` Bjorn Helgaas
2025-08-11 15:46 ` [PATCH v3 06/12] dt-bindings: PCI: Split exynos host into two files Shradha Todi
` (6 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Shradha Todi @ 2025-08-11 15:46 UTC (permalink / raw)
To: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey, Shradha Todi
Some resources might differ based on platforms and we need platform
specific functions to initialize or alter them. For better code
re-usability, making a separate res_ops which will hold all such
function pointers or other resource specific data. Include ops like
- init_regulator (initialize the regulator data)
- pcie_irq_handler (interrupt handler for PCIe)
- set_device_mode (set device mode to EP or RC)
Some operations maybe specific to certain SoCs and not applicable
to others. For such use cases, adding an SoC variant data field
which can be used to distinguish between the variants.
Some SoCs may have dual-role PCIe controller which can work as
RC or EP. Add device_mode to store the role and take decisions
accordingly.
Make enable/disable of regulator and initialization of IRQ as
common functions to be used by all Samsung SoCs.
Suggested-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
drivers/pci/controller/dwc/pci-exynos.c | 143 +++++++++++++++++++-----
1 file changed, 116 insertions(+), 27 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index c830b20d54f0..ef1f42236575 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -49,10 +49,18 @@
#define EXYNOS_PCIE_ELBI_SLV_ARMISC 0x120
#define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE BIT(21)
+/* to store different SoC variants of Samsung */
+enum samsung_pcie_variants {
+ EXYNOS_5433,
+};
+
struct samsung_pcie_pdata {
struct pci_ops *pci_ops;
const struct dw_pcie_ops *dwc_ops;
const struct dw_pcie_host_ops *host_ops;
+ const struct samsung_res_ops *res_ops;
+ unsigned int soc_variant;
+ enum dw_pcie_device_mode device_mode;
};
struct exynos_pcie {
@@ -61,7 +69,14 @@ struct exynos_pcie {
const struct samsung_pcie_pdata *pdata;
struct clk_bulk_data *clks;
struct phy *phy;
- struct regulator_bulk_data supplies[2];
+ struct regulator_bulk_data *supplies;
+ int supplies_cnt;
+};
+
+struct samsung_res_ops {
+ int (*init_regulator)(struct exynos_pcie *ep);
+ irqreturn_t (*pcie_irq_handler)(int irq, void *arg);
+ void (*set_device_mode)(struct exynos_pcie *ep);
};
static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
@@ -74,6 +89,31 @@ static u32 exynos_pcie_readl(void __iomem *base, u32 reg)
return readl(base + reg);
}
+static int samsung_regulator_enable(struct exynos_pcie *ep)
+{
+ int ret;
+
+ if (ep->supplies_cnt == 0)
+ return 0;
+
+ ret = regulator_bulk_enable(ep->supplies_cnt, ep->supplies);
+
+ return ret;
+}
+
+static void samsung_regulator_disable(struct exynos_pcie *ep)
+{
+ struct device *dev = ep->pci.dev;
+ int ret;
+
+ if (ep->supplies_cnt == 0)
+ return;
+
+ ret = regulator_bulk_disable(ep->supplies_cnt, ep->supplies);
+ if (ret)
+ dev_warn(dev, "failed to disable regulators: %d\n", ret);
+}
+
static void exynos_pcie_sideband_dbi_w_mode(struct exynos_pcie *ep, bool on)
{
u32 val;
@@ -244,7 +284,26 @@ static const struct dw_pcie_host_ops exynos_pcie_host_ops = {
.init = exynos_pcie_host_init,
};
-static int exynos_add_pcie_port(struct exynos_pcie *ep,
+static int exynos_init_regulator(struct exynos_pcie *ep)
+{
+ struct device *dev = ep->pci.dev;
+ int ret = 0;
+
+ ep->supplies_cnt = 2;
+
+ ep->supplies = devm_kcalloc(dev, ep->supplies_cnt, sizeof(*ep->supplies), GFP_KERNEL);
+ if (!ep->supplies)
+ return -ENOMEM;
+
+ ep->supplies[0].supply = "vdd18";
+ ep->supplies[1].supply = "vdd10";
+
+ ret = devm_regulator_bulk_get(dev, ep->supplies_cnt, ep->supplies);
+
+ return ret;
+}
+
+static int samsung_irq_init(struct exynos_pcie *ep,
struct platform_device *pdev)
{
struct dw_pcie *pci = &ep->pci;
@@ -256,22 +315,15 @@ static int exynos_add_pcie_port(struct exynos_pcie *ep,
if (pp->irq < 0)
return pp->irq;
- ret = devm_request_irq(dev, pp->irq, exynos_pcie_irq_handler,
+ ret = devm_request_irq(dev, pp->irq, ep->pdata->res_ops->pcie_irq_handler,
IRQF_SHARED, "exynos-pcie", ep);
if (ret) {
dev_err(dev, "failed to request irq\n");
return ret;
}
- pp->ops = &exynos_pcie_host_ops;
pp->msi_irq[0] = -ENODEV;
- ret = dw_pcie_host_init(pp);
- if (ret) {
- dev_err(dev, "failed to initialize host\n");
- return ret;
- }
-
return 0;
}
@@ -282,6 +334,11 @@ static const struct dw_pcie_ops exynos_dw_pcie_ops = {
.start_link = exynos_pcie_start_link,
};
+static const struct samsung_res_ops exynos_res_ops_data = {
+ .init_regulator = exynos_init_regulator,
+ .pcie_irq_handler = exynos_pcie_irq_handler,
+};
+
static int exynos_pcie_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -313,28 +370,46 @@ static int exynos_pcie_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
- ep->supplies[0].supply = "vdd18";
- ep->supplies[1].supply = "vdd10";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ep->supplies),
- ep->supplies);
- if (ret)
- return ret;
+ if (pdata->res_ops && pdata->res_ops->init_regulator) {
+ ret = ep->pdata->res_ops->init_regulator(ep);
+ if (ret)
+ return ret;
+ }
- ret = regulator_bulk_enable(ARRAY_SIZE(ep->supplies), ep->supplies);
+ ret = samsung_regulator_enable(ep);
if (ret)
return ret;
platform_set_drvdata(pdev, ep);
- ret = exynos_add_pcie_port(ep, pdev);
- if (ret < 0)
- goto fail_probe;
+ if (pdata->res_ops && pdata->res_ops->set_device_mode)
+ pdata->res_ops->set_device_mode(ep);
+
+ switch (ep->pdata->device_mode) {
+ case DW_PCIE_RC_TYPE:
+ ret = samsung_irq_init(ep, pdev);
+ if (ret)
+ goto fail_regulator;
+
+ ep->pci.pp.ops = pdata->host_ops;
+
+ ret = dw_pcie_host_init(&ep->pci.pp);
+ if (ret < 0)
+ goto fail_phy_init;
+
+ break;
+ default:
+ dev_err(dev, "invalid device type\n");
+ ret = -EINVAL;
+ goto fail_regulator;
+ }
return 0;
-fail_probe:
+fail_phy_init:
phy_exit(ep->phy);
- regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies);
+fail_regulator:
+ samsung_regulator_disable(ep);
return ret;
}
@@ -343,21 +418,29 @@ static void exynos_pcie_remove(struct platform_device *pdev)
{
struct exynos_pcie *ep = platform_get_drvdata(pdev);
+ if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
+ return;
dw_pcie_host_deinit(&ep->pci.pp);
- exynos_pcie_assert_core_reset(ep);
+ if (ep->pdata->soc_variant == EXYNOS_5433)
+ exynos_pcie_assert_core_reset(ep);
phy_power_off(ep->phy);
phy_exit(ep->phy);
- regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies);
+ samsung_regulator_disable(ep);
}
static int exynos_pcie_suspend_noirq(struct device *dev)
{
struct exynos_pcie *ep = dev_get_drvdata(dev);
+ struct dw_pcie *pci = &ep->pci;
- exynos_pcie_assert_core_reset(ep);
+ if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
+ return 0;
+
+ if (ep->pdata->soc_variant == EXYNOS_5433)
+ exynos_pcie_assert_core_reset(ep);
phy_power_off(ep->phy);
phy_exit(ep->phy);
- regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies);
+ samsung_regulator_disable(ep);
return 0;
}
@@ -369,7 +452,10 @@ static int exynos_pcie_resume_noirq(struct device *dev)
struct dw_pcie_rp *pp = &pci->pp;
int ret;
- ret = regulator_bulk_enable(ARRAY_SIZE(ep->supplies), ep->supplies);
+ if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
+ return 0;
+
+ ret = samsung_regulator_enable(ep);
if (ret)
return ret;
@@ -389,6 +475,9 @@ static const struct samsung_pcie_pdata exynos_5433_pcie_rc_pdata = {
.dwc_ops = &exynos_dw_pcie_ops,
.pci_ops = &exynos_pci_ops,
.host_ops = &exynos_pcie_host_ops,
+ .res_ops = &exynos_res_ops_data,
+ .soc_variant = EXYNOS_5433,
+ .device_mode = DW_PCIE_RC_TYPE,
};
static const struct of_device_id exynos_pcie_of_match[] = {
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 05/12] PCI: exynos: Add resource ops, soc variant and device mode
2025-08-11 15:46 ` [PATCH v3 05/12] PCI: exynos: Add resource ops, soc variant and device mode Shradha Todi
@ 2025-08-13 23:07 ` Bjorn Helgaas
2025-08-18 9:21 ` Shradha Todi
0 siblings, 1 reply; 34+ messages in thread
From: Bjorn Helgaas @ 2025-08-13 23:07 UTC (permalink / raw)
To: Shradha Todi
Cc: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy, mani, lpieralisi, kwilczynski, robh,
bhelgaas, jingoohan1, krzk+dt, conor+dt, alim.akhtar, vkoul,
kishon, arnd, m.szyprowski, jh80.chung, pankaj.dubey
On Mon, Aug 11, 2025 at 09:16:31PM +0530, Shradha Todi wrote:
> Some resources might differ based on platforms and we need platform
> specific functions to initialize or alter them. For better code
> re-usability, making a separate res_ops which will hold all such
> function pointers or other resource specific data. Include ops like
> - init_regulator (initialize the regulator data)
> - pcie_irq_handler (interrupt handler for PCIe)
> - set_device_mode (set device mode to EP or RC)
>
> Some operations maybe specific to certain SoCs and not applicable
> to others. For such use cases, adding an SoC variant data field
> which can be used to distinguish between the variants.
>
> Some SoCs may have dual-role PCIe controller which can work as
> RC or EP. Add device_mode to store the role and take decisions
> accordingly.
>
> Make enable/disable of regulator and initialization of IRQ as
> common functions to be used by all Samsung SoCs.
As hinted above, this patch ends up being a mixture of several things
that makes this kind of hard to review. Separating these into their
own patches would make it easier.
> Suggested-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
> drivers/pci/controller/dwc/pci-exynos.c | 143 +++++++++++++++++++-----
> 1 file changed, 116 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
> index c830b20d54f0..ef1f42236575 100644
> --- a/drivers/pci/controller/dwc/pci-exynos.c
> +++ b/drivers/pci/controller/dwc/pci-exynos.c
> @@ -49,10 +49,18 @@
> #define EXYNOS_PCIE_ELBI_SLV_ARMISC 0x120
> #define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE BIT(21)
>
> +/* to store different SoC variants of Samsung */
> +enum samsung_pcie_variants {
> + EXYNOS_5433,
> +};
> +
> struct samsung_pcie_pdata {
> struct pci_ops *pci_ops;
> const struct dw_pcie_ops *dwc_ops;
> const struct dw_pcie_host_ops *host_ops;
> + const struct samsung_res_ops *res_ops;
> + unsigned int soc_variant;
> + enum dw_pcie_device_mode device_mode;
> };
>
> struct exynos_pcie {
> @@ -61,7 +69,14 @@ struct exynos_pcie {
> const struct samsung_pcie_pdata *pdata;
> struct clk_bulk_data *clks;
> struct phy *phy;
> - struct regulator_bulk_data supplies[2];
> + struct regulator_bulk_data *supplies;
> + int supplies_cnt;
> +};
> +
> +struct samsung_res_ops {
> + int (*init_regulator)(struct exynos_pcie *ep);
> + irqreturn_t (*pcie_irq_handler)(int irq, void *arg);
> + void (*set_device_mode)(struct exynos_pcie *ep);
> };
>
> static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
> @@ -74,6 +89,31 @@ static u32 exynos_pcie_readl(void __iomem *base, u32 reg)
> return readl(base + reg);
> }
>
> +static int samsung_regulator_enable(struct exynos_pcie *ep)
> +{
> + int ret;
> +
> + if (ep->supplies_cnt == 0)
> + return 0;
> +
> + ret = regulator_bulk_enable(ep->supplies_cnt, ep->supplies);
> +
> + return ret;
Unless you want a warning here, there's no need for "ret":
return regulator_bulk_enable(ep->supplies_cnt, ep->supplies);
> +}
> +
> +static void samsung_regulator_disable(struct exynos_pcie *ep)
> +{
> + struct device *dev = ep->pci.dev;
> + int ret;
> +
> + if (ep->supplies_cnt == 0)
> + return;
> +
> + ret = regulator_bulk_disable(ep->supplies_cnt, ep->supplies);
> + if (ret)
> + dev_warn(dev, "failed to disable regulators: %d\n", ret);
> +}
> +
> static void exynos_pcie_sideband_dbi_w_mode(struct exynos_pcie *ep, bool on)
> {
> u32 val;
> @@ -244,7 +284,26 @@ static const struct dw_pcie_host_ops exynos_pcie_host_ops = {
> .init = exynos_pcie_host_init,
> };
>
> -static int exynos_add_pcie_port(struct exynos_pcie *ep,
> +static int exynos_init_regulator(struct exynos_pcie *ep)
> +{
> + struct device *dev = ep->pci.dev;
> + int ret = 0;
> +
> + ep->supplies_cnt = 2;
> +
> + ep->supplies = devm_kcalloc(dev, ep->supplies_cnt, sizeof(*ep->supplies), GFP_KERNEL);
> + if (!ep->supplies)
> + return -ENOMEM;
> +
> + ep->supplies[0].supply = "vdd18";
> + ep->supplies[1].supply = "vdd10";
> +
> + ret = devm_regulator_bulk_get(dev, ep->supplies_cnt, ep->supplies);
> +
> + return ret;
No need for ret.
> +}
> +
> +static int samsung_irq_init(struct exynos_pcie *ep,
> struct platform_device *pdev)
> {
> struct dw_pcie *pci = &ep->pci;
> @@ -256,22 +315,15 @@ static int exynos_add_pcie_port(struct exynos_pcie *ep,
> if (pp->irq < 0)
> return pp->irq;
>
> - ret = devm_request_irq(dev, pp->irq, exynos_pcie_irq_handler,
> + ret = devm_request_irq(dev, pp->irq, ep->pdata->res_ops->pcie_irq_handler,
> IRQF_SHARED, "exynos-pcie", ep);
> if (ret) {
> dev_err(dev, "failed to request irq\n");
> return ret;
> }
>
> - pp->ops = &exynos_pcie_host_ops;
> pp->msi_irq[0] = -ENODEV;
>
> - ret = dw_pcie_host_init(pp);
> - if (ret) {
> - dev_err(dev, "failed to initialize host\n");
> - return ret;
> - }
> -
> return 0;
> }
>
> @@ -282,6 +334,11 @@ static const struct dw_pcie_ops exynos_dw_pcie_ops = {
> .start_link = exynos_pcie_start_link,
> };
>
> +static const struct samsung_res_ops exynos_res_ops_data = {
> + .init_regulator = exynos_init_regulator,
> + .pcie_irq_handler = exynos_pcie_irq_handler,
> +};
> +
> static int exynos_pcie_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -313,28 +370,46 @@ static int exynos_pcie_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> - ep->supplies[0].supply = "vdd18";
> - ep->supplies[1].supply = "vdd10";
> - ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ep->supplies),
> - ep->supplies);
> - if (ret)
> - return ret;
> + if (pdata->res_ops && pdata->res_ops->init_regulator) {
> + ret = ep->pdata->res_ops->init_regulator(ep);
> + if (ret)
> + return ret;
> + }
>
> - ret = regulator_bulk_enable(ARRAY_SIZE(ep->supplies), ep->supplies);
> + ret = samsung_regulator_enable(ep);
> if (ret)
> return ret;
>
> platform_set_drvdata(pdev, ep);
>
> - ret = exynos_add_pcie_port(ep, pdev);
> - if (ret < 0)
> - goto fail_probe;
> + if (pdata->res_ops && pdata->res_ops->set_device_mode)
> + pdata->res_ops->set_device_mode(ep);
> +
> + switch (ep->pdata->device_mode) {
> + case DW_PCIE_RC_TYPE:
> + ret = samsung_irq_init(ep, pdev);
> + if (ret)
> + goto fail_regulator;
> +
> + ep->pci.pp.ops = pdata->host_ops;
> +
> + ret = dw_pcie_host_init(&ep->pci.pp);
> + if (ret < 0)
> + goto fail_phy_init;
> +
> + break;
> + default:
> + dev_err(dev, "invalid device type\n");
> + ret = -EINVAL;
> + goto fail_regulator;
> + }
>
> return 0;
>
> -fail_probe:
> +fail_phy_init:
> phy_exit(ep->phy);
> - regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies);
> +fail_regulator:
> + samsung_regulator_disable(ep);
>
> return ret;
> }
> @@ -343,21 +418,29 @@ static void exynos_pcie_remove(struct platform_device *pdev)
> {
> struct exynos_pcie *ep = platform_get_drvdata(pdev);
>
> + if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
> + return;
> dw_pcie_host_deinit(&ep->pci.pp);
> - exynos_pcie_assert_core_reset(ep);
> + if (ep->pdata->soc_variant == EXYNOS_5433)
> + exynos_pcie_assert_core_reset(ep);
I think it's a mistake to test for specific soc_variants. Better to
test a *property* (or use a function pointer than can be SoC-specific)
because it's likely that several SoCs will share the same property.
> phy_power_off(ep->phy);
> phy_exit(ep->phy);
> - regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies);
> + samsung_regulator_disable(ep);
> }
>
> static int exynos_pcie_suspend_noirq(struct device *dev)
> {
> struct exynos_pcie *ep = dev_get_drvdata(dev);
> + struct dw_pcie *pci = &ep->pci;
>
> - exynos_pcie_assert_core_reset(ep);
> + if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
> + return 0;
> +
> + if (ep->pdata->soc_variant == EXYNOS_5433)
> + exynos_pcie_assert_core_reset(ep);
> phy_power_off(ep->phy);
> phy_exit(ep->phy);
> - regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies);
> + samsung_regulator_disable(ep);
>
> return 0;
> }
> @@ -369,7 +452,10 @@ static int exynos_pcie_resume_noirq(struct device *dev)
> struct dw_pcie_rp *pp = &pci->pp;
> int ret;
>
> - ret = regulator_bulk_enable(ARRAY_SIZE(ep->supplies), ep->supplies);
> + if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
> + return 0;
> +
> + ret = samsung_regulator_enable(ep);
> if (ret)
> return ret;
>
> @@ -389,6 +475,9 @@ static const struct samsung_pcie_pdata exynos_5433_pcie_rc_pdata = {
> .dwc_ops = &exynos_dw_pcie_ops,
> .pci_ops = &exynos_pci_ops,
> .host_ops = &exynos_pcie_host_ops,
> + .res_ops = &exynos_res_ops_data,
> + .soc_variant = EXYNOS_5433,
> + .device_mode = DW_PCIE_RC_TYPE,
> };
>
> static const struct of_device_id exynos_pcie_of_match[] = {
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 34+ messages in thread* RE: [PATCH v3 05/12] PCI: exynos: Add resource ops, soc variant and device mode
2025-08-13 23:07 ` Bjorn Helgaas
@ 2025-08-18 9:21 ` Shradha Todi
0 siblings, 0 replies; 34+ messages in thread
From: Shradha Todi @ 2025-08-18 9:21 UTC (permalink / raw)
To: 'Bjorn Helgaas'
Cc: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy, mani, lpieralisi, kwilczynski, robh,
bhelgaas, jingoohan1, krzk+dt, conor+dt, alim.akhtar, vkoul,
kishon, arnd, m.szyprowski, jh80.chung, pankaj.dubey
> > Some resources might differ based on platforms and we need platform
> > specific functions to initialize or alter them. For better code
> > re-usability, making a separate res_ops which will hold all such
> > function pointers or other resource specific data. Include ops like
> > - init_regulator (initialize the regulator data)
> > - pcie_irq_handler (interrupt handler for PCIe)
> > - set_device_mode (set device mode to EP or RC)
> >
> > Some operations maybe specific to certain SoCs and not applicable
> > to others. For such use cases, adding an SoC variant data field
> > which can be used to distinguish between the variants.
> >
> > Some SoCs may have dual-role PCIe controller which can work as
> > RC or EP. Add device_mode to store the role and take decisions
> > accordingly.
> >
> > Make enable/disable of regulator and initialization of IRQ as
> > common functions to be used by all Samsung SoCs.
>
> As hinted above, this patch ends up being a mixture of several things
> that makes this kind of hard to review. Separating these into their
> own patches would make it easier.
>
Will split into multiple patches
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 06/12] dt-bindings: PCI: Split exynos host into two files
2025-08-11 15:46 ` [PATCH v3 00/12] Add PCIe support for Tesla FSD SoC Shradha Todi
` (4 preceding siblings ...)
2025-08-11 15:46 ` [PATCH v3 05/12] PCI: exynos: Add resource ops, soc variant and device mode Shradha Todi
@ 2025-08-11 15:46 ` Shradha Todi
2025-08-12 6:32 ` Krzysztof Kozlowski
2025-08-11 15:46 ` [PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC Shradha Todi
` (5 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Shradha Todi @ 2025-08-11 15:46 UTC (permalink / raw)
To: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey, Shradha Todi
The current Exynos PCIe yaml binding file is hard to reuse by
other Samsung SoCs. Refactoring it by:
- Moving common Samsung PCIe properties into samsung,exynos-pcie.yaml
- Creating a dedicated samsung,exynos5433-pcie.yaml file for properties
and constraints specific to the Exynos5433 SoC
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
.../bindings/pci/samsung,exynos-pcie.yaml | 70 +--------------
.../bindings/pci/samsung,exynos5433-pcie.yaml | 89 +++++++++++++++++++
2 files changed, 91 insertions(+), 68 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos5433-pcie.yaml
diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
index f20ed7e709f7..fd0b97b30821 100644
--- a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
@@ -11,7 +11,7 @@ maintainers:
- Jaehoon Chung <jh80.chung@samsung.com>
description: |+
- Exynos5433 SoC PCIe host controller is based on the Synopsys DesignWare
+ Samsung SoCs PCIe host controller is based on the Synopsys DesignWare
PCIe IP and thus inherits all the common properties defined in
snps,dw-pcie.yaml.
@@ -19,9 +19,6 @@ allOf:
- $ref: /schemas/pci/snps,dw-pcie.yaml#
properties:
- compatible:
- const: samsung,exynos5433-pcie
-
reg:
items:
- description: Data Bus Interface (DBI) registers.
@@ -37,83 +34,20 @@ properties:
interrupts:
maxItems: 1
- clocks:
- items:
- - description: PCIe bridge clock
- - description: PCIe bus clock
-
- clock-names:
- items:
- - const: pcie
- - const: pcie_bus
-
phys:
maxItems: 1
- vdd10-supply:
- description:
- Phandle to a regulator that provides 1.0V power to the PCIe block.
-
- vdd18-supply:
- description:
- Phandle to a regulator that provides 1.8V power to the PCIe block.
-
- num-lanes:
- const: 1
-
- num-viewport:
- const: 3
-
required:
- reg
- reg-names
- interrupts
- "#address-cells"
- "#size-cells"
- - "#interrupt-cells"
- - interrupt-map
- - interrupt-map-mask
- ranges
- - bus-range
- device_type
- num-lanes
- - num-viewport
- clocks
- clock-names
- phys
- - vdd10-supply
- - vdd18-supply
-
-unevaluatedProperties: false
-
-examples:
- - |
- #include <dt-bindings/interrupt-controller/irq.h>
- #include <dt-bindings/interrupt-controller/arm-gic.h>
- #include <dt-bindings/clock/exynos5433.h>
- pcie: pcie@15700000 {
- compatible = "samsung,exynos5433-pcie";
- reg = <0x15700000 0x1000>, <0x156b0000 0x1000>, <0x0c000000 0x1000>;
- reg-names = "dbi", "elbi", "config";
- #address-cells = <3>;
- #size-cells = <2>;
- #interrupt-cells = <1>;
- device_type = "pci";
- interrupts = <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&cmu_fsys CLK_PCIE>, <&cmu_fsys CLK_PCLK_PCIE_PHY>;
- clock-names = "pcie", "pcie_bus";
- phys = <&pcie_phy>;
- pinctrl-0 = <&pcie_bus &pcie_wlanen>;
- pinctrl-names = "default";
- num-lanes = <1>;
- num-viewport = <3>;
- bus-range = <0x00 0xff>;
- ranges = <0x81000000 0 0 0x0c001000 0 0x00010000>,
- <0x82000000 0 0x0c011000 0x0c011000 0 0x03feefff>;
- vdd10-supply = <&ldo6_reg>;
- vdd18-supply = <&ldo7_reg>;
- interrupt-map-mask = <0 0 0 0>;
- interrupt-map = <0 0 0 0 &gic GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
- };
-...
+additionalProperties: true
diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5433-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos5433-pcie.yaml
new file mode 100644
index 000000000000..1fb2c32899c4
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/samsung,exynos5433-pcie.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/samsung,exynos5433-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung Exynos5433 SoC PCIe Host Controller
+
+maintainers:
+ - Marek Szyprowski <m.szyprowski@samsung.com>
+ - Jaehoon Chung <jh80.chung@samsung.com>
+
+description:
+ Exynos5433 SoCs PCIe host controller inherits all the
+ common properties defined in samsung,exynos-pcie.yaml
+
+allOf:
+ - $ref: /schemas/pci/samsung,exynos-pcie.yaml#
+
+properties:
+ compatible:
+ const: samsung,exynos5433-pcie
+
+ clocks:
+ items:
+ - description: PCIe bridge clock
+ - description: PCIe bus clock
+
+ clock-names:
+ items:
+ - const: pcie
+ - const: pcie_bus
+
+ num-lanes:
+ const: 1
+
+ num-viewport:
+ const: 3
+
+ vdd10-supply:
+ description:
+ phandle to a regulator that provides 1.0v power to the pcie block
+
+ vdd18-supply:
+ description:
+ phandle to a regulator that provides 1.8v power to the pcie block
+
+required:
+ - "#interrupt-cells"
+ - interrupt-map
+ - interrupt-map-mask
+ - bus-range
+ - num-viewport
+ - vdd10-supply
+ - vdd18-supply
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/exynos5433.h>
+
+ pcie: pcie@15700000 {
+ compatible = "samsung,exynos5433-pcie";
+ reg = <0x15700000 0x1000>, <0x156b0000 0x1000>, <0x0c000000 0x1000>;
+ reg-names = "dbi", "elbi", "config";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+ device_type = "pci";
+ interrupts = <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cmu_fsys CLK_PCIE>, <&cmu_fsys CLK_PCLK_PCIE_PHY>;
+ clock-names = "pcie", "pcie_bus";
+ phys = <&pcie_phy>;
+ pinctrl-0 = <&pcie_bus &pcie_wlanen>;
+ pinctrl-names = "default";
+ num-lanes = <1>;
+ num-viewport = <3>;
+ bus-range = <0x00 0xff>;
+ ranges = <0x81000000 0 0 0x0c001000 0 0x00010000>,
+ <0x82000000 0 0x0c011000 0x0c011000 0 0x03feefff>;
+ vdd10-supply = <&ldo6_reg>;
+ vdd18-supply = <&ldo7_reg>;
+ interrupt-map-mask = <0 0 0 0>;
+ interrupt-map = <0 0 0 0 &gic GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
+ };
+...
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 06/12] dt-bindings: PCI: Split exynos host into two files
2025-08-11 15:46 ` [PATCH v3 06/12] dt-bindings: PCI: Split exynos host into two files Shradha Todi
@ 2025-08-12 6:32 ` Krzysztof Kozlowski
2025-08-18 8:41 ` Shradha Todi
0 siblings, 1 reply; 34+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-12 6:32 UTC (permalink / raw)
To: Shradha Todi, linux-pci, devicetree, linux-arm-kernel,
linux-samsung-soc, linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey
On 11/08/2025 17:46, Shradha Todi wrote:
> The current Exynos PCIe yaml binding file is hard to reuse by
> other Samsung SoCs. Refactoring it by:
> - Moving common Samsung PCIe properties into samsung,exynos-pcie.yaml
> - Creating a dedicated samsung,exynos5433-pcie.yaml file for properties
> and constraints specific to the Exynos5433 SoC
>
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
> .../bindings/pci/samsung,exynos-pcie.yaml | 70 +--------------
> .../bindings/pci/samsung,exynos5433-pcie.yaml | 89 +++++++++++++++++++
> 2 files changed, 91 insertions(+), 68 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos5433-pcie.yaml
>
> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> index f20ed7e709f7..fd0b97b30821 100644
> --- a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> @@ -11,7 +11,7 @@ maintainers:
> - Jaehoon Chung <jh80.chung@samsung.com>
>
> description: |+
> - Exynos5433 SoC PCIe host controller is based on the Synopsys DesignWare
> + Samsung SoCs PCIe host controller is based on the Synopsys DesignWare
> PCIe IP and thus inherits all the common properties defined in
> snps,dw-pcie.yaml.
>
> @@ -19,9 +19,6 @@ allOf:
> - $ref: /schemas/pci/snps,dw-pcie.yaml#
>
> properties:
> - compatible:
> - const: samsung,exynos5433-pcie
> -
> reg:
> items:
> - description: Data Bus Interface (DBI) registers.
So the only common part left here is reg and phy? I don't think such
common file brings any value.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCH v3 06/12] dt-bindings: PCI: Split exynos host into two files
2025-08-12 6:32 ` Krzysztof Kozlowski
@ 2025-08-18 8:41 ` Shradha Todi
0 siblings, 0 replies; 34+ messages in thread
From: Shradha Todi @ 2025-08-18 8:41 UTC (permalink / raw)
To: 'Krzysztof Kozlowski', linux-pci, devicetree,
linux-arm-kernel, linux-samsung-soc, linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey
> >
> > @@ -19,9 +19,6 @@ allOf:
> > - $ref: /schemas/pci/snps,dw-pcie.yaml#
> >
> > properties:
> > - compatible:
> > - const: samsung,exynos5433-pcie
> > -
> > reg:
> > items:
> > - description: Data Bus Interface (DBI) registers.
>
>
> So the only common part left here is reg and phy? I don't think such
> common file brings any value.
>
>
Okay, will keep two separate files.
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC
2025-08-11 15:46 ` [PATCH v3 00/12] Add PCIe support for Tesla FSD SoC Shradha Todi
` (5 preceding siblings ...)
2025-08-11 15:46 ` [PATCH v3 06/12] dt-bindings: PCI: Split exynos host into two files Shradha Todi
@ 2025-08-11 15:46 ` Shradha Todi
2025-08-12 6:37 ` Krzysztof Kozlowski
2025-08-30 3:27 ` Manivannan Sadhasivam
2025-08-11 15:46 ` [PATCH v3 08/12] dt-bindings: phy: Add PCIe PHY support for " Shradha Todi
` (4 subsequent siblings)
11 siblings, 2 replies; 34+ messages in thread
From: Shradha Todi @ 2025-08-11 15:46 UTC (permalink / raw)
To: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey, Shradha Todi
Add Tesla FSD SoC support for both RC and EP.
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
.../bindings/pci/tesla,fsd-pcie-ep.yaml | 91 +++++++++++++++++++
.../bindings/pci/tesla,fsd-pcie.yaml | 77 ++++++++++++++++
2 files changed, 168 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/tesla,fsd-pcie-ep.yaml
create mode 100644 Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml
diff --git a/Documentation/devicetree/bindings/pci/tesla,fsd-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/tesla,fsd-pcie-ep.yaml
new file mode 100644
index 000000000000..8dfe0720e6ab
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/tesla,fsd-pcie-ep.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/tesla,fsd-pcie-ep.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Tesla FSD SoC series PCIe Endpoint Controller
+
+maintainers:
+ - Shradha Todi <shradha.t@samsung.com>
+
+description:
+ Tesla FSD SoCs PCIe endpoint controller is based on the Synopsys DesignWare
+ PCIe IP and thus inherits all the common properties defined in
+ snps,dw-pcie-ep.yaml
+
+allOf:
+ - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
+
+properties:
+ compatible:
+ const: tesla,fsd-pcie-ep
+
+ reg:
+ maxItems: 4
+
+ reg-names:
+ items:
+ - const: elbi
+ - const: dbi
+ - const: dbi2
+ - const: addr_space
+
+ clocks:
+ maxItems: 4
+
+ clock-names:
+ items:
+ - const: aux
+ - const: dbi
+ - const: mstr
+ - const: slv
+
+ num-lanes:
+ maximum: 4
+
+ phys:
+ maxItems: 1
+
+ samsung,syscon-pcie:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: phandle for system control registers, used to
+ control signals at system level
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - clocks
+ - clock-names
+ - num-lanes
+ - phys
+ - samsung,syscon-pcie
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/fsd-clk.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ bus {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ pcieep0: pcie-ep@16a00000 {
+ compatible = "tesla,fsd-pcie-ep";
+ reg = <0x0 0x168b0000 0x0 0x1000>,
+ <0x0 0x16a00000 0x0 0x2000>,
+ <0x0 0x16a01000 0x0 0x80>,
+ <0x0 0x17000000 0x0 0xff0000>;
+ reg-names = "elbi", "dbi", "dbi2", "addr_space";
+ clocks = <&clock_fsys1 PCIE_LINK0_IPCLKPORT_AUX_ACLK>,
+ <&clock_fsys1 PCIE_LINK0_IPCLKPORT_DBI_ACLK>,
+ <&clock_fsys1 PCIE_LINK0_IPCLKPORT_MSTR_ACLK>,
+ <&clock_fsys1 PCIE_LINK0_IPCLKPORT_SLV_ACLK>;
+ clock-names = "aux", "dbi", "mstr", "slv";
+ num-lanes = <4>;
+ phys = <&pciephy1>;
+ samsung,syscon-pcie = <&sysreg_fsys1 0x50c>;
+ };
+ };
+...
diff --git a/Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml b/Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml
new file mode 100644
index 000000000000..533870ab1d73
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/tesla,fsd-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Tesla FSD SoC series PCIe Host Controller
+
+maintainers:
+ - Shradha Todi <shradha.t@samsung.com>
+
+description:
+ Tesla FSD SoCs PCIe host controller inherits all the common
+ properties defined in samsung,exynos-pcie.yaml
+
+allOf:
+ - $ref: /schemas/pci/samsung,exynos-pcie.yaml#
+
+properties:
+ compatible:
+ const: tesla,fsd-pcie
+
+ clocks:
+ maxItems: 4
+
+ clock-names:
+ items:
+ - const: aux
+ - const: dbi
+ - const: mstr
+ - const: slv
+
+ num-lanes:
+ maximum: 4
+
+ samsung,syscon-pcie:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: phandle for system control registers, used to
+ control signals at system level
+
+required:
+ - samsung,syscon-pcie
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/fsd-clk.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ pcierc1: pcie@16b00000 {
+ compatible = "tesla,fsd-pcie";
+ reg = <0x0 0x16b00000 0x0 0x2000>,
+ <0x0 0x168c0000 0x0 0x1000>,
+ <0x0 0x18000000 0x0 0x1000>;
+ reg-names = "dbi", "elbi", "config";
+ ranges = <0x82000000 0x0 0x18001000 0x0 0x18001000 0x0 0xffefff>;
+ clocks = <&clock_fsys1 PCIE_LINK1_IPCLKPORT_AUX_ACLK>,
+ <&clock_fsys1 PCIE_LINK1_IPCLKPORT_DBI_ACLK>,
+ <&clock_fsys1 PCIE_LINK1_IPCLKPORT_MSTR_ACLK>,
+ <&clock_fsys1 PCIE_LINK1_IPCLKPORT_SLV_ACLK>;
+ clock-names = "aux", "dbi", "mstr", "slv";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ dma-coherent;
+ device_type = "pci";
+ interrupts = <GIC_SPI 117 IRQ_TYPE_EDGE_RISING>;
+ num-lanes = <4>;
+ phys = <&pciephy1>;
+ samsung,syscon-pcie = <&sysreg_fsys1 0x510>;
+ };
+ };
+...
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC
2025-08-11 15:46 ` [PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC Shradha Todi
@ 2025-08-12 6:37 ` Krzysztof Kozlowski
2025-08-18 8:46 ` Shradha Todi
2025-08-30 3:27 ` Manivannan Sadhasivam
1 sibling, 1 reply; 34+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-12 6:37 UTC (permalink / raw)
To: Shradha Todi, linux-pci, devicetree, linux-arm-kernel,
linux-samsung-soc, linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey
On 11/08/2025 17:46, Shradha Todi wrote:
> +
> + clocks:
> + maxItems: 4
> +
> + clock-names:
> + items:
> + - const: aux
> + - const: dbi
> + - const: mstr
> + - const: slv
> +
> + num-lanes:
> + maximum: 4
> +
> + phys:
> + maxItems: 1
> +
> + samsung,syscon-pcie:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: phandle for system control registers, used to
> + control signals at system level
What is "system level"? and what are these "signals" being controlled?
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - clocks
> + - clock-names
> + - num-lanes
> + - phys
> + - samsung,syscon-pcie
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/fsd-clk.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + bus {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + pcieep0: pcie-ep@16a00000 {
> + compatible = "tesla,fsd-pcie-ep";
> + reg = <0x0 0x168b0000 0x0 0x1000>,
> + <0x0 0x16a00000 0x0 0x2000>,
> + <0x0 0x16a01000 0x0 0x80>,
> + <0x0 0x17000000 0x0 0xff0000>;
> + reg-names = "elbi", "dbi", "dbi2", "addr_space";
> + clocks = <&clock_fsys1 PCIE_LINK0_IPCLKPORT_AUX_ACLK>,
> + <&clock_fsys1 PCIE_LINK0_IPCLKPORT_DBI_ACLK>,
> + <&clock_fsys1 PCIE_LINK0_IPCLKPORT_MSTR_ACLK>,
> + <&clock_fsys1 PCIE_LINK0_IPCLKPORT_SLV_ACLK>;
> + clock-names = "aux", "dbi", "mstr", "slv";
> + num-lanes = <4>;
> + phys = <&pciephy1>;
> + samsung,syscon-pcie = <&sysreg_fsys1 0x50c>;
> + };
> + };
> +...
> diff --git a/Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml b/Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml
> new file mode 100644
> index 000000000000..533870ab1d73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/tesla,fsd-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Tesla FSD SoC series PCIe Host Controller
> +
> +maintainers:
> + - Shradha Todi <shradha.t@samsung.com>
> +
> +description:
> + Tesla FSD SoCs PCIe host controller inherits all the common
> + properties defined in samsung,exynos-pcie.yaml
> +
> +allOf:
> + - $ref: /schemas/pci/samsung,exynos-pcie.yaml#
> +
> +properties:
> + compatible:
> + const: tesla,fsd-pcie
> +
> + clocks:
> + maxItems: 4
> +
> + clock-names:
> + items:
> + - const: aux
> + - const: dbi
> + - const: mstr
> + - const: slv
> +
> + num-lanes:
> + maximum: 4
> +
> + samsung,syscon-pcie:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: phandle for system control registers, used to
> + control signals at system level
> +
> +required:
> + - samsung,syscon-pcie
clocks are required, compatible as well.
Missing supplies, both as properties and required. PCI devices do not
work without power.
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/fsd-clk.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + pcierc1: pcie@16b00000 {
> + compatible = "tesla,fsd-pcie";
> + reg = <0x0 0x16b00000 0x0 0x2000>,
> + <0x0 0x168c0000 0x0 0x1000>,
> + <0x0 0x18000000 0x0 0x1000>;
> + reg-names = "dbi", "elbi", "config";
> + ranges = <0x82000000 0x0 0x18001000 0x0 0x18001000 0x0 0xffefff>;
Misaligned. Follow closely DTS coding style.
> + clocks = <&clock_fsys1 PCIE_LINK1_IPCLKPORT_AUX_ACLK>,
> + <&clock_fsys1 PCIE_LINK1_IPCLKPORT_DBI_ACLK>,
> + <&clock_fsys1 PCIE_LINK1_IPCLKPORT_MSTR_ACLK>,
> + <&clock_fsys1 PCIE_LINK1_IPCLKPORT_SLV_ACLK>;
> + clock-names = "aux", "dbi", "mstr", "slv";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + dma-coherent;
> + device_type = "pci";
> + interrupts = <GIC_SPI 117 IRQ_TYPE_EDGE_RISING>;
> + num-lanes = <4>;
> + phys = <&pciephy1>;
> + samsung,syscon-pcie = <&sysreg_fsys1 0x510>;
Incomplete, missing supplies.
> + };
> + };
> +...
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 34+ messages in thread* RE: [PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC
2025-08-12 6:37 ` Krzysztof Kozlowski
@ 2025-08-18 8:46 ` Shradha Todi
2025-08-30 3:21 ` Manivannan Sadhasivam
0 siblings, 1 reply; 34+ messages in thread
From: Shradha Todi @ 2025-08-18 8:46 UTC (permalink / raw)
To: 'Krzysztof Kozlowski', linux-pci, devicetree,
linux-arm-kernel, linux-samsung-soc, linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey
> > +
> > + phys:
> > + maxItems: 1
> > +
> > + samsung,syscon-pcie:
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > + description: phandle for system control registers, used to
> > + control signals at system level
>
> What is "system level"? and what are these "signals" being controlled?
>
I will add a more detailed description for why the syscon is being used
>
> > +title: Tesla FSD SoC series PCIe Host Controller
> > +
> > +maintainers:
> > + - Shradha Todi <shradha.t@samsung.com>
> > +
> > +description:
> > + Tesla FSD SoCs PCIe host controller inherits all the common
> > + properties defined in samsung,exynos-pcie.yaml
> > +
> > +allOf:
> > + - $ref: /schemas/pci/samsung,exynos-pcie.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: tesla,fsd-pcie
> > +
> > + clocks:
> > + maxItems: 4
> > +
> > + clock-names:
> > + items:
> > + - const: aux
> > + - const: dbi
> > + - const: mstr
> > + - const: slv
> > +
> > + num-lanes:
> > + maximum: 4
> > +
> > + samsung,syscon-pcie:
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > + description: phandle for system control registers, used to
> > + control signals at system level
> > +
> > +required:
> > + - samsung,syscon-pcie
>
> clocks are required, compatible as well.
>
Since this was inheriting the common exynos yaml file and that had these properties
under required, I did not mention again. Will take care in next version.
> Missing supplies, both as properties and required. PCI devices do not
> work without power.
>
According to the HW design of FSD SoC, the control to manage PCIe power is given to
a separate CPU where custom firmware runs. Therefore, the Linux side does not control
the PCIe power supplies directly and are hence not included in the device tree.
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/fsd-clk.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > + soc {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + pcierc1: pcie@16b00000 {
> > + compatible = "tesla,fsd-pcie";
> > + reg = <0x0 0x16b00000 0x0 0x2000>,
> > + <0x0 0x168c0000 0x0 0x1000>,
> > + <0x0 0x18000000 0x0 0x1000>;
> > + reg-names = "dbi", "elbi", "config";
> > + ranges = <0x82000000 0x0 0x18001000 0x0 0x18001000 0x0 0xffefff>;
>
> Misaligned. Follow closely DTS coding style.
>
Will take care.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC
2025-08-18 8:46 ` Shradha Todi
@ 2025-08-30 3:21 ` Manivannan Sadhasivam
0 siblings, 0 replies; 34+ messages in thread
From: Manivannan Sadhasivam @ 2025-08-30 3:21 UTC (permalink / raw)
To: Shradha Todi
Cc: 'Krzysztof Kozlowski', linux-pci, devicetree,
linux-arm-kernel, linux-samsung-soc, linux-kernel, linux-phy,
lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1, krzk+dt,
conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey
On Mon, Aug 18, 2025 at 02:16:16PM GMT, Shradha Todi wrote:
> > > +
> > > + phys:
> > > + maxItems: 1
> > > +
> > > + samsung,syscon-pcie:
> > > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > > + description: phandle for system control registers, used to
> > > + control signals at system level
> >
> > What is "system level"? and what are these "signals" being controlled?
> >
>
> I will add a more detailed description for why the syscon is being used
>
> >
> > > +title: Tesla FSD SoC series PCIe Host Controller
> > > +
> > > +maintainers:
> > > + - Shradha Todi <shradha.t@samsung.com>
> > > +
> > > +description:
> > > + Tesla FSD SoCs PCIe host controller inherits all the common
> > > + properties defined in samsung,exynos-pcie.yaml
> > > +
> > > +allOf:
> > > + - $ref: /schemas/pci/samsung,exynos-pcie.yaml#
> > > +
> > > +properties:
> > > + compatible:
> > > + const: tesla,fsd-pcie
> > > +
> > > + clocks:
> > > + maxItems: 4
> > > +
> > > + clock-names:
> > > + items:
> > > + - const: aux
> > > + - const: dbi
> > > + - const: mstr
> > > + - const: slv
> > > +
> > > + num-lanes:
> > > + maximum: 4
> > > +
> > > + samsung,syscon-pcie:
> > > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > > + description: phandle for system control registers, used to
> > > + control signals at system level
> > > +
> > > +required:
> > > + - samsung,syscon-pcie
> >
> > clocks are required, compatible as well.
> >
>
> Since this was inheriting the common exynos yaml file and that had these properties
> under required, I did not mention again. Will take care in next version.
>
dma-coherent needs to be a required property as well since this binding is
supporting only one controller, that seem to have cache coherent DMA.
> > Missing supplies, both as properties and required. PCI devices do not
> > work without power.
> >
>
> According to the HW design of FSD SoC, the control to manage PCIe power is given to
> a separate CPU where custom firmware runs. Therefore, the Linux side does not control
> the PCIe power supplies directly and are hence not included in the device tree.
What do you mean by 'PCIe power'? Supply to the PCIe controller/bus or the
devices connected to the bus?
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC
2025-08-11 15:46 ` [PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC Shradha Todi
2025-08-12 6:37 ` Krzysztof Kozlowski
@ 2025-08-30 3:27 ` Manivannan Sadhasivam
1 sibling, 0 replies; 34+ messages in thread
From: Manivannan Sadhasivam @ 2025-08-30 3:27 UTC (permalink / raw)
To: Shradha Todi
Cc: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy, lpieralisi, kwilczynski, robh, bhelgaas,
jingoohan1, krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd,
m.szyprowski, jh80.chung, pankaj.dubey
On Mon, Aug 11, 2025 at 09:16:33PM GMT, Shradha Todi wrote:
> Add Tesla FSD SoC support for both RC and EP.
Add some info about the PCIe controller here. Like the data rate supported,
lanes, interrupts, any quirks etc...
>
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
> .../bindings/pci/tesla,fsd-pcie-ep.yaml | 91 +++++++++++++++++++
> .../bindings/pci/tesla,fsd-pcie.yaml | 77 ++++++++++++++++
> 2 files changed, 168 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/tesla,fsd-pcie-ep.yaml
> create mode 100644 Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml
>
[...]
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/fsd-clk.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + pcierc1: pcie@16b00000 {
> + compatible = "tesla,fsd-pcie";
> + reg = <0x0 0x16b00000 0x0 0x2000>,
> + <0x0 0x168c0000 0x0 0x1000>,
> + <0x0 0x18000000 0x0 0x1000>;
> + reg-names = "dbi", "elbi", "config";
> + ranges = <0x82000000 0x0 0x18001000 0x0 0x18001000 0x0 0xffefff>;
> + clocks = <&clock_fsys1 PCIE_LINK1_IPCLKPORT_AUX_ACLK>,
> + <&clock_fsys1 PCIE_LINK1_IPCLKPORT_DBI_ACLK>,
> + <&clock_fsys1 PCIE_LINK1_IPCLKPORT_MSTR_ACLK>,
> + <&clock_fsys1 PCIE_LINK1_IPCLKPORT_SLV_ACLK>;
> + clock-names = "aux", "dbi", "mstr", "slv";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + dma-coherent;
> + device_type = "pci";
> + interrupts = <GIC_SPI 117 IRQ_TYPE_EDGE_RISING>;
Only one SPI interrupt? What about INTx? Don't you have any external/internal
MSI controller?
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 08/12] dt-bindings: phy: Add PCIe PHY support for FSD SoC
2025-08-11 15:46 ` [PATCH v3 00/12] Add PCIe support for Tesla FSD SoC Shradha Todi
` (6 preceding siblings ...)
2025-08-11 15:46 ` [PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC Shradha Todi
@ 2025-08-11 15:46 ` Shradha Todi
2025-08-14 8:13 ` Krzysztof Kozlowski
2025-08-11 15:46 ` [PATCH v3 09/12] phy: exynos: Add platform device private data Shradha Todi
` (3 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Shradha Todi @ 2025-08-11 15:46 UTC (permalink / raw)
To: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey, Shradha Todi
Since Tesla FSD SoC uses Samsung PCIe PHY, add support in
exynos PCIe PHY bindings.
In Tesla FSD SoC, the two PHY instances, although having identical
hardware design and register maps, are placed in different locations
(Placement and routing) inside the SoC and have distinct
PHY-to-Controller topologies. (One instance is connected to two PCIe
controllers, while the other is connected to only one). As a result,
they experience different analog environments, including varying
channel losses and noise profiles.
Since these PHYs lack internal adaptation mechanisms and f/w based
tuning, manual register programming is required for analog tuning.
To ensure optimal signal integrity, it is essential to use different
register values for each PHY instance, despite their identical hardware
design. This is because the same register values may not be suitable
for both instances due to their differing environments and topologies.
Due to this, we are using two PHY compatibles for different PHY
instances.
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
.../bindings/phy/samsung,exynos-pcie-phy.yaml | 27 +++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml
index 41df8bb08ff7..6295472696db 100644
--- a/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml
@@ -15,10 +15,14 @@ properties:
const: 0
compatible:
- const: samsung,exynos5433-pcie-phy
+ enum:
+ - samsung,exynos5433-pcie-phy
+ - tesla,fsd-pcie-phy0
+ - tesla,fsd-pcie-phy1
reg:
- maxItems: 1
+ minItems: 1
+ maxItems: 2
samsung,pmu-syscon:
$ref: /schemas/types.yaml#/definitions/phandle
@@ -30,6 +34,25 @@ properties:
description: phandle for FSYS sysreg interface, used to control
sysreg registers bits for PCIe PHY
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - tesla,fsd-pcie-phy0
+ - tesla,fsd-pcie-phy1
+ then:
+ properties:
+ reg:
+ items:
+ - description: PHY
+ - description: PCS
+ else:
+ properties:
+ reg:
+ maxItems: 1
+
required:
- "#phy-cells"
- compatible
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 08/12] dt-bindings: phy: Add PCIe PHY support for FSD SoC
2025-08-11 15:46 ` [PATCH v3 08/12] dt-bindings: phy: Add PCIe PHY support for " Shradha Todi
@ 2025-08-14 8:13 ` Krzysztof Kozlowski
0 siblings, 0 replies; 34+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-14 8:13 UTC (permalink / raw)
To: Shradha Todi
Cc: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy, mani, lpieralisi, kwilczynski, robh,
bhelgaas, jingoohan1, krzk+dt, conor+dt, alim.akhtar, vkoul,
kishon, arnd, m.szyprowski, jh80.chung, pankaj.dubey
On Mon, Aug 11, 2025 at 09:16:34PM +0530, Shradha Todi wrote:
> Since Tesla FSD SoC uses Samsung PCIe PHY, add support in
> exynos PCIe PHY bindings.
>
> In Tesla FSD SoC, the two PHY instances, although having identical
> hardware design and register maps, are placed in different locations
> (Placement and routing) inside the SoC and have distinct
> PHY-to-Controller topologies. (One instance is connected to two PCIe
> controllers, while the other is connected to only one). As a result,
> they experience different analog environments, including varying
> channel losses and noise profiles.
>
> Since these PHYs lack internal adaptation mechanisms and f/w based
> tuning, manual register programming is required for analog tuning.
> To ensure optimal signal integrity, it is essential to use different
> register values for each PHY instance, despite their identical hardware
> design. This is because the same register values may not be suitable
> for both instances due to their differing environments and topologies.
Would be nice if above (or most of it) would be reflected in binding
description. Please do so and:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 09/12] phy: exynos: Add platform device private data
2025-08-11 15:46 ` [PATCH v3 00/12] Add PCIe support for Tesla FSD SoC Shradha Todi
` (7 preceding siblings ...)
2025-08-11 15:46 ` [PATCH v3 08/12] dt-bindings: phy: Add PCIe PHY support for " Shradha Todi
@ 2025-08-11 15:46 ` Shradha Todi
2025-08-11 15:46 ` [PATCH v3 10/12] phy: exynos: Add PCIe PHY support for FSD SoC Shradha Todi
` (2 subsequent siblings)
11 siblings, 0 replies; 34+ messages in thread
From: Shradha Todi @ 2025-08-11 15:46 UTC (permalink / raw)
To: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey, Shradha Todi
Currently, the exynos PCIe PHY driver supports only single
phy ops making it unusable for other platforms. Add the phy_ops
as platform specific device data so as to extend this driver
to support all platforms using Samsung PHY.
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
drivers/phy/samsung/phy-exynos-pcie.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/samsung/phy-exynos-pcie.c b/drivers/phy/samsung/phy-exynos-pcie.c
index 53c9230c2907..022222a0212a 100644
--- a/drivers/phy/samsung/phy-exynos-pcie.c
+++ b/drivers/phy/samsung/phy-exynos-pcie.c
@@ -136,6 +136,7 @@ static const struct phy_ops exynos5433_phy_ops = {
static const struct of_device_id exynos_pcie_phy_match[] = {
{
.compatible = "samsung,exynos5433-pcie-phy",
+ .data = &exynos5433_phy_ops,
},
{},
};
@@ -146,6 +147,11 @@ static int exynos_pcie_phy_probe(struct platform_device *pdev)
struct exynos_pcie_phy *exynos_phy;
struct phy *generic_phy;
struct phy_provider *phy_provider;
+ const struct phy_ops *drv_data;
+
+ drv_data = of_device_get_match_data(dev);
+ if (!drv_data)
+ return -ENODEV;
exynos_phy = devm_kzalloc(dev, sizeof(*exynos_phy), GFP_KERNEL);
if (!exynos_phy)
@@ -169,7 +175,7 @@ static int exynos_pcie_phy_probe(struct platform_device *pdev)
return PTR_ERR(exynos_phy->fsysreg);
}
- generic_phy = devm_phy_create(dev, dev->of_node, &exynos5433_phy_ops);
+ generic_phy = devm_phy_create(dev, dev->of_node, drv_data);
if (IS_ERR(generic_phy)) {
dev_err(dev, "failed to create PHY\n");
return PTR_ERR(generic_phy);
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v3 10/12] phy: exynos: Add PCIe PHY support for FSD SoC
2025-08-11 15:46 ` [PATCH v3 00/12] Add PCIe support for Tesla FSD SoC Shradha Todi
` (8 preceding siblings ...)
2025-08-11 15:46 ` [PATCH v3 09/12] phy: exynos: Add platform device private data Shradha Todi
@ 2025-08-11 15:46 ` Shradha Todi
2025-09-01 12:11 ` Vinod Koul
2025-08-11 15:46 ` [PATCH v3 11/12] PCI: exynos: Add support for Tesla " Shradha Todi
2025-08-11 15:46 ` [PATCH v3 12/12] arm64: dts: fsd: Add PCIe " Shradha Todi
11 siblings, 1 reply; 34+ messages in thread
From: Shradha Todi @ 2025-08-11 15:46 UTC (permalink / raw)
To: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey, Shradha Todi
Add PCIe PHY support for Tesla FSD SoC.
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
drivers/phy/samsung/phy-exynos-pcie.c | 287 ++++++++++++++++++++++++++
1 file changed, 287 insertions(+)
diff --git a/drivers/phy/samsung/phy-exynos-pcie.c b/drivers/phy/samsung/phy-exynos-pcie.c
index 022222a0212a..5a55a22f9661 100644
--- a/drivers/phy/samsung/phy-exynos-pcie.c
+++ b/drivers/phy/samsung/phy-exynos-pcie.c
@@ -34,9 +34,105 @@
/* PMU PCIE PHY isolation control */
#define EXYNOS5433_PMU_PCIE_PHY_OFFSET 0x730
+/* FSD: PCIe PHY common registers */
+#define FSD_PCIE_PHY_TRSV_CMN_REG03 0x000c
+#define FSD_PCIE_PHY_TRSV_CMN_REG01E 0x0078
+#define FSD_PCIE_PHY_TRSV_CMN_REG02D 0x00b4
+#define FSD_PCIE_PHY_TRSV_CMN_REG031 0x00c4
+#define FSD_PCIE_PHY_TRSV_CMN_REG036 0x00d8
+#define FSD_PCIE_PHY_TRSV_CMN_REG05F 0x017c
+#define FSD_PCIE_PHY_TRSV_CMN_REG060 0x0180
+#define FSD_PCIE_PHY_TRSV_CMN_REG062 0x0188
+#define FSD_PCIE_PHY_TRSV_CMN_REG061 0x0184
+#define FSD_PCIE_PHY_AGG_BIF_RESET 0x0200
+#define FSD_PCIE_PHY_AGG_BIF_CLOCK 0x0208
+#define FSD_PCIE_PHY_CMN_RESET 0x0228
+
+/* FSD: PCIe PHY lane registers */
+#define FSD_PCIE_LANE_OFFSET 0x0400
+#define FSD_PCIE_NUM_LANES 0x4
+
+#define FSD_PCIE_PHY_TRSV_REG001_LN_N 0x0404
+#define FSD_PCIE_PHY_TRSV_REG002_LN_N 0x0408
+#define FSD_PCIE_PHY_TRSV_REG005_LN_N 0x0414
+#define FSD_PCIE_PHY_TRSV_REG006_LN_N 0x0418
+#define FSD_PCIE_PHY_TRSV_REG007_LN_N 0x041c
+#define FSD_PCIE_PHY_TRSV_REG009_LN_N 0x0424
+#define FSD_PCIE_PHY_TRSV_REG00A_LN_N 0x0428
+#define FSD_PCIE_PHY_TRSV_REG00C_LN_N 0x0430
+#define FSD_PCIE_PHY_TRSV_REG012_LN_N 0x0448
+#define FSD_PCIE_PHY_TRSV_REG013_LN_N 0x044c
+#define FSD_PCIE_PHY_TRSV_REG014_LN_N 0x0450
+#define FSD_PCIE_PHY_TRSV_REG015_LN_N 0x0454
+#define FSD_PCIE_PHY_TRSV_REG016_LN_N 0x0458
+#define FSD_PCIE_PHY_TRSV_REG018_LN_N 0x0460
+#define FSD_PCIE_PHY_TRSV_REG020_LN_N 0x0480
+#define FSD_PCIE_PHY_TRSV_REG026_LN_N 0x0498
+#define FSD_PCIE_PHY_TRSV_REG029_LN_N 0x04a4
+#define FSD_PCIE_PHY_TRSV_REG031_LN_N 0x04c4
+#define FSD_PCIE_PHY_TRSV_REG036_LN_N 0x04d8
+#define FSD_PCIE_PHY_TRSV_REG039_LN_N 0x04e4
+#define FSD_PCIE_PHY_TRSV_REG03B_LN_N 0x04ec
+#define FSD_PCIE_PHY_TRSV_REG03C_LN_N 0x04f0
+#define FSD_PCIE_PHY_TRSV_REG03E_LN_N 0x04f8
+#define FSD_PCIE_PHY_TRSV_REG03F_LN_N 0x04fc
+#define FSD_PCIE_PHY_TRSV_REG043_LN_N 0x050c
+#define FSD_PCIE_PHY_TRSV_REG044_LN_N 0x0510
+#define FSD_PCIE_PHY_TRSV_REG046_LN_N 0x0518
+#define FSD_PCIE_PHY_TRSV_REG048_LN_N 0x0520
+#define FSD_PCIE_PHY_TRSV_REG049_LN_N 0x0524
+#define FSD_PCIE_PHY_TRSV_REG04E_LN_N 0x0538
+#define FSD_PCIE_PHY_TRSV_REG052_LN_N 0x0548
+#define FSD_PCIE_PHY_TRSV_REG068_LN_N 0x05a0
+#define FSD_PCIE_PHY_TRSV_REG069_LN_N 0x05a4
+#define FSD_PCIE_PHY_TRSV_REG06A_LN_N 0x05a8
+#define FSD_PCIE_PHY_TRSV_REG06B_LN_N 0x05ac
+#define FSD_PCIE_PHY_TRSV_REG07B_LN_N 0x05ec
+#define FSD_PCIE_PHY_TRSV_REG083_LN_N 0x060c
+#define FSD_PCIE_PHY_TRSV_REG084_LN_N 0x0610
+#define FSD_PCIE_PHY_TRSV_REG086_LN_N 0x0618
+#define FSD_PCIE_PHY_TRSV_REG087_LN_N 0x061c
+#define FSD_PCIE_PHY_TRSV_REG08B_LN_N 0x062c
+#define FSD_PCIE_PHY_TRSV_REG09C_LN_N 0x0670
+#define FSD_PCIE_PHY_TRSV_REG09D_LN_N 0x0674
+#define FSD_PCIE_PHY_TRSV_REG09E_LN_N 0x0678
+#define FSD_PCIE_PHY_TRSV_REG09F_LN_N 0x067c
+#define FSD_PCIE_PHY_TRSV_REG0A2_LN_N 0x0688
+#define FSD_PCIE_PHY_TRSV_REG0A4_LN_N 0x0690
+#define FSD_PCIE_PHY_TRSV_REG0CE_LN_N 0x0738
+#define FSD_PCIE_PHY_TRSV_REG0FC_LN_N 0x07f0
+#define FSD_PCIE_PHY_TRSV_REG0FD_LN_N 0x07f4
+#define FSD_PCIE_PHY_TRSV_REG0FE_LN_N 0x07f8
+#define FSD_PCIE_PHY_TRSV_REG0CE_LN_1 0x0b38
+#define FSD_PCIE_PHY_TRSV_REG0CE_LN_2 0x0f38
+#define FSD_PCIE_PHY_TRSV_REG0CE_LN_3 0x1338
+
+/* FSD: PCIe PCS registers */
+#define FSD_PCIE_PCS_BRF_0 0x0004
+#define FSD_PCIE_PCS_BRF_1 0x0804
+#define FSD_PCIE_PCS_CLK 0x0180
+
+/* FSD: PCIe SYSREG registers */
+#define FSD_PCIE_SYSREG_PHY_0_CON 0x042c
+#define FSD_PCIE_SYSREG_PHY_0_CON_MASK 0x03ff
+#define FSD_PCIE_SYSREG_PHY_0_REF_SEL (0x2 << 0)
+#define FSD_PCIE_SYSREG_PHY_0_REF_SEL_MASK 0x3
+#define FSD_PCIE_SYSREG_PHY_0_AUX_EN BIT(4)
+#define FSD_PCIE_SYSREG_PHY_0_CMN_RSTN BIT(8)
+#define FSD_PCIE_SYSREG_PHY_0_INIT_RSTN BIT(9)
+
+#define FSD_PCIE_SYSREG_PHY_1_CON 0x0500
+#define FSD_PCIE_SYSREG_PHY_1_CON_MASK 0x01ff
+#define FSD_PCIE_SYSREG_PHY_1_REF_SEL (0x2 << 4)
+#define FSD_PCIE_SYSREG_PHY_1_REF_SEL_MASK 0x30
+#define FSD_PCIE_SYSREG_PHY_1_AUX_EN BIT(0)
+#define FSD_PCIE_SYSREG_PHY_1_CMN_RSTN BIT(1)
+#define FSD_PCIE_SYSREG_PHY_1_INIT_RSTN BIT(3)
+
/* For Exynos pcie phy */
struct exynos_pcie_phy {
void __iomem *base;
+ void __iomem *pcs_base;
struct regmap *pmureg;
struct regmap *fsysreg;
};
@@ -133,11 +229,200 @@ static const struct phy_ops exynos5433_phy_ops = {
.owner = THIS_MODULE,
};
+static void fsd_pcie_phy_writel(struct exynos_pcie_phy *phy_ctrl, u32 offset, u32 val)
+{
+ void __iomem *phy_base = phy_ctrl->base;
+ u32 i;
+
+ for (i = 0; i < FSD_PCIE_NUM_LANES; i++)
+ writel(val, phy_base + (offset + i * FSD_PCIE_LANE_OFFSET));
+}
+
+static int fsd_pcie_phy0_reset(struct phy *phy)
+{
+ struct exynos_pcie_phy *phy_ctrl = phy_get_drvdata(phy);
+
+ writel(0x1, phy_ctrl->pcs_base + FSD_PCIE_PCS_CLK);
+
+ regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
+ FSD_PCIE_SYSREG_PHY_0_CON_MASK, 0x0);
+ regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
+ FSD_PCIE_SYSREG_PHY_0_AUX_EN, FSD_PCIE_SYSREG_PHY_0_AUX_EN);
+ regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
+ FSD_PCIE_SYSREG_PHY_0_REF_SEL_MASK, FSD_PCIE_SYSREG_PHY_0_REF_SEL);
+ regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
+ FSD_PCIE_SYSREG_PHY_0_INIT_RSTN, FSD_PCIE_SYSREG_PHY_0_INIT_RSTN);
+
+ return 0;
+}
+
+static int fsd_pcie_phy1_reset(struct phy *phy)
+{
+ struct exynos_pcie_phy *phy_ctrl = phy_get_drvdata(phy);
+
+ writel(0x1, phy_ctrl->pcs_base + FSD_PCIE_PCS_CLK);
+
+ regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_1_CON,
+ FSD_PCIE_SYSREG_PHY_1_CON_MASK, 0x0);
+ regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_1_CON,
+ FSD_PCIE_SYSREG_PHY_1_AUX_EN, FSD_PCIE_SYSREG_PHY_1_AUX_EN);
+ regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_1_CON,
+ FSD_PCIE_SYSREG_PHY_1_REF_SEL_MASK, FSD_PCIE_SYSREG_PHY_1_REF_SEL);
+ regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_1_CON,
+ FSD_PCIE_SYSREG_PHY_1_INIT_RSTN, FSD_PCIE_SYSREG_PHY_1_INIT_RSTN);
+
+ return 0;
+}
+
+static int fsd_pcie_phy0_init(struct phy *phy)
+{
+ struct exynos_pcie_phy *phy_ctrl = phy_get_drvdata(phy);
+ void __iomem *pbase = phy_ctrl->base;
+
+ fsd_pcie_phy0_reset(phy);
+
+ writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_0);
+ writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_1);
+ writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_RESET);
+ writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_CLOCK);
+
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG07B_LN_N, 0x20);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG052_LN_N, 0x00);
+ writel(0x11, pbase + FSD_PCIE_PHY_TRSV_CMN_REG05F);
+ writel(0x23, pbase + FSD_PCIE_PHY_TRSV_CMN_REG060);
+ writel(0x0, pbase + FSD_PCIE_PHY_TRSV_CMN_REG062);
+ writel(0x15, pbase + FSD_PCIE_PHY_TRSV_CMN_REG03);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG0CE_LN_N, 0x8);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG039_LN_N, 0xf);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03B_LN_N, 0x13);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03C_LN_N, 0xf6);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG044_LN_N, 0x57);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03E_LN_N, 0x10);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03F_LN_N, 0x04);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG043_LN_N, 0x11);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG049_LN_N, 0x6f);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG04E_LN_N, 0x18);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG068_LN_N, 0x1f);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG069_LN_N, 0xc);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG06B_LN_N, 0x78);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG083_LN_N, 0xa);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG084_LN_N, 0x80);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG086_LN_N, 0xff);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG087_LN_N, 0x3c);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG09D_LN_N, 0x7c);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG09E_LN_N, 0x33);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG09F_LN_N, 0x33);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG001_LN_N, 0x3f);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG002_LN_N, 0x1c);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG005_LN_N, 0x2b);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG006_LN_N, 0x3);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG007_LN_N, 0x0c);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG009_LN_N, 0x10);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG00A_LN_N, 0x1);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG00C_LN_N, 0x93);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG012_LN_N, 0x1);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG013_LN_N, 0x0);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG014_LN_N, 0x70);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG015_LN_N, 0x0);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG016_LN_N, 0x70);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG0FC_LN_N, 0x80);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG0FD_LN_N, 0x0);
+
+ regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
+ FSD_PCIE_SYSREG_PHY_0_CMN_RSTN, FSD_PCIE_SYSREG_PHY_0_CMN_RSTN);
+
+ return 0;
+}
+
+static int fsd_pcie_phy1_init(struct phy *phy)
+{
+ struct exynos_pcie_phy *phy_ctrl = phy_get_drvdata(phy);
+ void __iomem *pbase = phy_ctrl->base;
+
+ fsd_pcie_phy1_reset(phy);
+
+ writel(0x2, pbase + FSD_PCIE_PHY_CMN_RESET);
+
+ writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_0);
+ writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_1);
+ writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_RESET);
+ writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_CLOCK);
+
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG07B_LN_N, 0x20);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG052_LN_N, 0x00);
+ writel(0xaa, pbase + FSD_PCIE_PHY_TRSV_CMN_REG01E);
+ writel(0x28, pbase + FSD_PCIE_PHY_TRSV_CMN_REG02D);
+ writel(0x28, pbase + FSD_PCIE_PHY_TRSV_CMN_REG031);
+ writel(0x21, pbase + FSD_PCIE_PHY_TRSV_CMN_REG036);
+ writel(0x12, pbase + FSD_PCIE_PHY_TRSV_CMN_REG05F);
+ writel(0x23, pbase + FSD_PCIE_PHY_TRSV_CMN_REG060);
+ writel(0x0, pbase + FSD_PCIE_PHY_TRSV_CMN_REG061);
+ writel(0x0, pbase + FSD_PCIE_PHY_TRSV_CMN_REG062);
+ writel(0x15, pbase + FSD_PCIE_PHY_TRSV_CMN_REG03);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG039_LN_N, 0xf);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03B_LN_N, 0x13);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03C_LN_N, 0x66);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG044_LN_N, 0x57);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03E_LN_N, 0x10);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03F_LN_N, 0x44);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG043_LN_N, 0x11);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG046_LN_N, 0xef);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG048_LN_N, 0x06);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG049_LN_N, 0xaf);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG04E_LN_N, 0x28);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG068_LN_N, 0x1f);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG069_LN_N, 0xc);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG06A_LN_N, 0x8);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG06B_LN_N, 0x78);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG083_LN_N, 0xa);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG084_LN_N, 0x80);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG087_LN_N, 0x30);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG08B_LN_N, 0xa0);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG09C_LN_N, 0xf7);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG09E_LN_N, 0x33);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG0A2_LN_N, 0xfa);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG0A4_LN_N, 0xf2);
+ writel(0x8, pbase + FSD_PCIE_PHY_TRSV_REG0CE_LN_N);
+ writel(0x9, pbase + FSD_PCIE_PHY_TRSV_REG0CE_LN_1);
+ writel(0x9, pbase + FSD_PCIE_PHY_TRSV_REG0CE_LN_2);
+ writel(0x9, pbase + FSD_PCIE_PHY_TRSV_REG0CE_LN_3);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG0FE_LN_N, 0x33);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG001_LN_N, 0x3f);
+ fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG005_LN_N, 0x2b);
+
+ writel(0x3, pbase + FSD_PCIE_PHY_CMN_RESET);
+
+ regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_1_CON,
+ FSD_PCIE_SYSREG_PHY_1_CMN_RSTN, FSD_PCIE_SYSREG_PHY_1_CMN_RSTN);
+
+ return 0;
+}
+
+static const struct phy_ops fsd_phy0_ops = {
+ .init = fsd_pcie_phy0_init,
+ .reset = fsd_pcie_phy0_reset,
+ .owner = THIS_MODULE,
+};
+
+static const struct phy_ops fsd_phy1_ops = {
+ .init = fsd_pcie_phy1_init,
+ .reset = fsd_pcie_phy1_reset,
+ .owner = THIS_MODULE,
+};
+
static const struct of_device_id exynos_pcie_phy_match[] = {
{
.compatible = "samsung,exynos5433-pcie-phy",
.data = &exynos5433_phy_ops,
},
+ {
+ .compatible = "tesla,fsd-pcie-phy0",
+ .data = &fsd_phy0_ops,
+ },
+ {
+ .compatible = "tesla,fsd-pcie-phy1",
+ .data = &fsd_phy1_ops,
+ },
{},
};
@@ -181,6 +466,8 @@ static int exynos_pcie_phy_probe(struct platform_device *pdev)
return PTR_ERR(generic_phy);
}
+ exynos_phy->pcs_base = devm_platform_ioremap_resource(pdev, 1);
+
phy_set_drvdata(generic_phy, exynos_phy);
phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 10/12] phy: exynos: Add PCIe PHY support for FSD SoC
2025-08-11 15:46 ` [PATCH v3 10/12] phy: exynos: Add PCIe PHY support for FSD SoC Shradha Todi
@ 2025-09-01 12:11 ` Vinod Koul
0 siblings, 0 replies; 34+ messages in thread
From: Vinod Koul @ 2025-09-01 12:11 UTC (permalink / raw)
To: Shradha Todi
Cc: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy, mani, lpieralisi, kwilczynski, robh,
bhelgaas, jingoohan1, krzk+dt, conor+dt, alim.akhtar, kishon,
arnd, m.szyprowski, jh80.chung, pankaj.dubey
On 11-08-25, 21:16, Shradha Todi wrote:
> Add PCIe PHY support for Tesla FSD SoC.
Can you pls add a bit more description of what you are adding, helps to
understand the change
> +/* FSD: PCIe PCS registers */
> +#define FSD_PCIE_PCS_BRF_0 0x0004
> +#define FSD_PCIE_PCS_BRF_1 0x0804
> +#define FSD_PCIE_PCS_CLK 0x0180
> +
> +/* FSD: PCIe SYSREG registers */
> +#define FSD_PCIE_SYSREG_PHY_0_CON 0x042c
> +#define FSD_PCIE_SYSREG_PHY_0_CON_MASK 0x03ff
> +#define FSD_PCIE_SYSREG_PHY_0_REF_SEL (0x2 << 0)
Use GENMASK() please here and elsewhere
> +static int fsd_pcie_phy0_reset(struct phy *phy)
> +{
> + struct exynos_pcie_phy *phy_ctrl = phy_get_drvdata(phy);
> +
> + writel(0x1, phy_ctrl->pcs_base + FSD_PCIE_PCS_CLK);
> +
> + regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
> + FSD_PCIE_SYSREG_PHY_0_CON_MASK, 0x0);
> + regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
> + FSD_PCIE_SYSREG_PHY_0_AUX_EN, FSD_PCIE_SYSREG_PHY_0_AUX_EN);
> + regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
> + FSD_PCIE_SYSREG_PHY_0_REF_SEL_MASK, FSD_PCIE_SYSREG_PHY_0_REF_SEL);
> + regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
> + FSD_PCIE_SYSREG_PHY_0_INIT_RSTN, FSD_PCIE_SYSREG_PHY_0_INIT_RSTN);
pls conform to coding style for these
> +
> + return 0;
why return a value when this wont ever return anything else than 0?
> +
> + writel(0x2, pbase + FSD_PCIE_PHY_CMN_RESET);
> +
> + writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_0);
> + writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_1);
> + writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_RESET);
> + writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_CLOCK);
> +
> + fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG07B_LN_N, 0x20);
> + fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG052_LN_N, 0x00);
> + writel(0xaa, pbase + FSD_PCIE_PHY_TRSV_CMN_REG01E);
> + writel(0x28, pbase + FSD_PCIE_PHY_TRSV_CMN_REG02D);
> + writel(0x28, pbase + FSD_PCIE_PHY_TRSV_CMN_REG031);
> + writel(0x21, pbase + FSD_PCIE_PHY_TRSV_CMN_REG036);
> + writel(0x12, pbase + FSD_PCIE_PHY_TRSV_CMN_REG05F);
> + writel(0x23, pbase + FSD_PCIE_PHY_TRSV_CMN_REG060);
> + writel(0x0, pbase + FSD_PCIE_PHY_TRSV_CMN_REG061);
> + writel(0x0, pbase + FSD_PCIE_PHY_TRSV_CMN_REG062);
> + writel(0x15, pbase + FSD_PCIE_PHY_TRSV_CMN_REG03);
Magic numbers?
--
~Vinod
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC
2025-08-11 15:46 ` [PATCH v3 00/12] Add PCIe support for Tesla FSD SoC Shradha Todi
` (9 preceding siblings ...)
2025-08-11 15:46 ` [PATCH v3 10/12] phy: exynos: Add PCIe PHY support for FSD SoC Shradha Todi
@ 2025-08-11 15:46 ` Shradha Todi
2025-08-13 23:07 ` Bjorn Helgaas
2025-08-30 3:54 ` Manivannan Sadhasivam
2025-08-11 15:46 ` [PATCH v3 12/12] arm64: dts: fsd: Add PCIe " Shradha Todi
11 siblings, 2 replies; 34+ messages in thread
From: Shradha Todi @ 2025-08-11 15:46 UTC (permalink / raw)
To: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey, Shradha Todi
Add host and endpoint controller driver support for FSD SoC.
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
drivers/pci/controller/dwc/pci-exynos.c | 278 ++++++++++++++++++++++++
1 file changed, 278 insertions(+)
diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index ef1f42236575..9aabfecdc147 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -18,6 +18,8 @@
#include <linux/platform_device.h>
#include <linux/phy/phy.h>
#include <linux/regulator/consumer.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
@@ -49,15 +51,35 @@
#define EXYNOS_PCIE_ELBI_SLV_ARMISC 0x120
#define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE BIT(21)
+#define FSD_IRQ2_STS 0x008
+#define FSD_IRQ_MSI_ENABLE BIT(17)
+#define FSD_IRQ2_EN 0x018
+#define FSD_PCIE_APP_LTSSM_ENABLE 0x054
+#define FSD_PCIE_LTSSM_ENABLE 0x1
+#define FSD_PCIE_DEVICE_TYPE 0x080
+#define FSD_DEVICE_TYPE_RC 0x4
+#define FSD_DEVICE_TYPE_EP 0x0
+#define FSD_PCIE_CXPL_DEBUG_00_31 0x2c8
+
/* to store different SoC variants of Samsung */
enum samsung_pcie_variants {
+ FSD,
EXYNOS_5433,
};
+/* Values to be written to SYSREG to view DBI space as CDM/DBI2/IATU/DMA */
+enum fsd_pcie_addr_type {
+ ADDR_TYPE_DBI = 0x0,
+ ADDR_TYPE_DBI2 = 0x12,
+ ADDR_TYPE_ATU = 0x36,
+ ADDR_TYPE_DMA = 0x3f,
+};
+
struct samsung_pcie_pdata {
struct pci_ops *pci_ops;
const struct dw_pcie_ops *dwc_ops;
const struct dw_pcie_host_ops *host_ops;
+ const struct dw_pcie_ep_ops *ep_ops;
const struct samsung_res_ops *res_ops;
unsigned int soc_variant;
enum dw_pcie_device_mode device_mode;
@@ -67,6 +89,8 @@ struct exynos_pcie {
struct dw_pcie pci;
void __iomem *elbi_base;
const struct samsung_pcie_pdata *pdata;
+ struct regmap *sysreg;
+ unsigned int sysreg_offset;
struct clk_bulk_data *clks;
struct phy *phy;
struct regulator_bulk_data *supplies;
@@ -334,11 +358,201 @@ static const struct dw_pcie_ops exynos_dw_pcie_ops = {
.start_link = exynos_pcie_start_link,
};
+static void fsd_pcie_stop_link(struct dw_pcie *pci)
+{
+ u32 val;
+ struct exynos_pcie *ep = to_exynos_pcie(pci);
+
+ val = readl(ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
+ val &= ~FSD_PCIE_LTSSM_ENABLE;
+ writel(val, ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
+}
+
+static int fsd_pcie_start_link(struct dw_pcie *pci)
+{
+ struct exynos_pcie *ep = to_exynos_pcie(pci);
+ struct dw_pcie_ep *dw_ep = &pci->ep;
+
+ if (dw_pcie_link_up(pci))
+ return 0;
+
+ writel(FSD_PCIE_LTSSM_ENABLE, ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
+
+ /* no need to wait for link in case of host as core files take care */
+ if (ep->pdata->device_mode == DW_PCIE_RC_TYPE)
+ return 0;
+
+ /* check if the link is up or not in case of EP */
+ if (!dw_pcie_wait_for_link(pci)) {
+ dw_pcie_ep_linkup(dw_ep);
+ return 0;
+ }
+
+ return -ETIMEDOUT;
+}
+
+static irqreturn_t fsd_pcie_irq_handler(int irq, void *arg)
+{
+ u32 val;
+ struct exynos_pcie *ep = arg;
+ struct dw_pcie *pci = &ep->pci;
+ struct dw_pcie_rp *pp = &pci->pp;
+
+ val = readl(ep->elbi_base + FSD_IRQ2_STS);
+ if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
+ val &= FSD_IRQ_MSI_ENABLE;
+ writel(val, ep->elbi_base + FSD_IRQ2_STS);
+ dw_handle_msi_irq(pp);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static void fsd_pcie_msi_init(struct exynos_pcie *ep)
+{
+ int val;
+
+ val = readl(ep->elbi_base + FSD_IRQ2_EN);
+ val |= FSD_IRQ_MSI_ENABLE;
+ writel(val, ep->elbi_base + FSD_IRQ2_EN);
+}
+
+static void __iomem *fsd_atu_setting(struct dw_pcie *pci, void __iomem *base)
+{
+ struct exynos_pcie *ep = to_exynos_pcie(pci);
+
+ if (base >= pci->atu_base) {
+ regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_ATU);
+ return (base - DEFAULT_DBI_ATU_OFFSET);
+ } else if (base == pci->dbi_base) {
+ regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
+ } else if (base == pci->dbi_base2) {
+ regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI2);
+ }
+
+ return base;
+}
+
+static u32 fsd_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
+ u32 reg, size_t size)
+{
+ void __iomem *addr;
+ u32 val;
+
+ addr = fsd_atu_setting(pci, base);
+ dw_pcie_read(addr + reg, size, &val);
+
+ return val;
+}
+
+static void fsd_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
+ u32 reg, size_t size, u32 val)
+{
+ void __iomem *addr;
+
+ addr = fsd_atu_setting(pci, base);
+ dw_pcie_write(addr + reg, size, val);
+}
+
+static void fsd_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base,
+ u32 reg, size_t size, u32 val)
+{
+ struct exynos_pcie *ep = to_exynos_pcie(pci);
+
+ fsd_atu_setting(pci, base);
+ dw_pcie_write(pci->dbi_base + reg, size, val);
+ regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
+}
+
+static bool fsd_pcie_link_up(struct dw_pcie *pci)
+{
+ u32 val;
+ struct exynos_pcie *ep = to_exynos_pcie(pci);
+
+ val = readl(ep->elbi_base + FSD_PCIE_CXPL_DEBUG_00_31);
+ val &= PORT_LOGIC_LTSSM_STATE_MASK;
+
+ return (val == PORT_LOGIC_LTSSM_STATE_L0);
+}
+
+static int fsd_pcie_host_init(struct dw_pcie_rp *pp)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct exynos_pcie *ep = to_exynos_pcie(pci);
+
+ phy_init(ep->phy);
+ fsd_pcie_msi_init(ep);
+
+ return 0;
+}
+
+static const struct dw_pcie_host_ops fsd_pcie_host_ops = {
+ .init = fsd_pcie_host_init,
+};
+
+static int fsd_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
+ unsigned int type, u16 interrupt_num)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+
+ switch (type) {
+ case PCI_IRQ_INTX:
+ return dw_pcie_ep_raise_intx_irq(ep, func_no);
+ case PCI_IRQ_MSIX:
+ dev_err(pci->dev, "EP does not support MSI-X\n");
+ return -EINVAL;
+ case PCI_IRQ_MSI:
+ return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
+ default:
+ dev_err(pci->dev, "UNKNOWN IRQ type\n");
+ }
+
+ return 0;
+}
+
+static const struct pci_epc_features fsd_pcie_epc_features = {
+ .linkup_notifier = false,
+ .msi_capable = true,
+ .msix_capable = false,
+};
+
+static const struct pci_epc_features *fsd_pcie_get_features(struct dw_pcie_ep *ep)
+{
+ return &fsd_pcie_epc_features;
+}
+
+static const struct dw_pcie_ep_ops fsd_ep_ops = {
+ .raise_irq = fsd_pcie_raise_irq,
+ .get_features = fsd_pcie_get_features,
+};
+
+static void fsd_set_device_mode(struct exynos_pcie *ep)
+{
+ if (ep->pdata->device_mode == DW_PCIE_RC_TYPE)
+ writel(FSD_DEVICE_TYPE_RC, ep->elbi_base + FSD_PCIE_DEVICE_TYPE);
+ else
+ writel(FSD_DEVICE_TYPE_EP, ep->elbi_base + FSD_PCIE_DEVICE_TYPE);
+}
+
+static const struct dw_pcie_ops fsd_dw_pcie_ops = {
+ .read_dbi = fsd_pcie_read_dbi,
+ .write_dbi = fsd_pcie_write_dbi,
+ .write_dbi2 = fsd_pcie_write_dbi2,
+ .start_link = fsd_pcie_start_link,
+ .stop_link = fsd_pcie_stop_link,
+ .link_up = fsd_pcie_link_up,
+};
+
static const struct samsung_res_ops exynos_res_ops_data = {
.init_regulator = exynos_init_regulator,
.pcie_irq_handler = exynos_pcie_irq_handler,
};
+static const struct samsung_res_ops fsd_res_ops_data = {
+ .pcie_irq_handler = fsd_pcie_irq_handler,
+ .set_device_mode = fsd_set_device_mode,
+};
+
static int exynos_pcie_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -361,6 +575,26 @@ static int exynos_pcie_probe(struct platform_device *pdev)
if (IS_ERR(ep->phy))
return PTR_ERR(ep->phy);
+ if (ep->pdata->soc_variant == FSD) {
+ ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
+ if (ret)
+ return ret;
+
+ ep->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
+ "samsung,syscon-pcie");
+ if (IS_ERR(ep->sysreg)) {
+ dev_err(dev, "sysreg regmap lookup failed.\n");
+ return PTR_ERR(ep->sysreg);
+ }
+
+ ret = of_property_read_u32_index(dev->of_node, "samsung,syscon-pcie", 1,
+ &ep->sysreg_offset);
+ if (ret) {
+ dev_err(dev, "couldn't get the register offset for syscon!\n");
+ return ret;
+ }
+ }
+
/* External Local Bus interface (ELBI) registers */
ep->elbi_base = devm_platform_ioremap_resource_byname(pdev, "elbi");
if (IS_ERR(ep->elbi_base))
@@ -397,6 +631,22 @@ static int exynos_pcie_probe(struct platform_device *pdev)
if (ret < 0)
goto fail_phy_init;
+ break;
+ case DW_PCIE_EP_TYPE:
+ phy_init(ep->phy);
+
+ ep->pci.ep.ops = pdata->ep_ops;
+
+ ret = dw_pcie_ep_init(&ep->pci.ep);
+ if (ret < 0)
+ goto fail_phy_init;
+
+ ret = dw_pcie_ep_init_registers(&ep->pci.ep);
+ if (ret)
+ goto fail_phy_init;
+
+ pci_epc_init_notify(ep->pci.ep.epc);
+
break;
default:
dev_err(dev, "invalid device type\n");
@@ -436,6 +686,9 @@ static int exynos_pcie_suspend_noirq(struct device *dev)
if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
return 0;
+ if (ep->pdata->dwc_ops->stop_link)
+ ep->pdata->dwc_ops->stop_link(pci);
+
if (ep->pdata->soc_variant == EXYNOS_5433)
exynos_pcie_assert_core_reset(ep);
phy_power_off(ep->phy);
@@ -471,6 +724,23 @@ static const struct dev_pm_ops exynos_pcie_pm_ops = {
exynos_pcie_resume_noirq)
};
+
+static const struct samsung_pcie_pdata fsd_hw3_pcie_rc_pdata = {
+ .dwc_ops = &fsd_dw_pcie_ops,
+ .host_ops = &fsd_pcie_host_ops,
+ .res_ops = &fsd_res_ops_data,
+ .soc_variant = FSD,
+ .device_mode = DW_PCIE_RC_TYPE,
+};
+
+static const struct samsung_pcie_pdata fsd_hw3_pcie_ep_pdata = {
+ .dwc_ops = &fsd_dw_pcie_ops,
+ .ep_ops = &fsd_ep_ops,
+ .res_ops = &fsd_res_ops_data,
+ .soc_variant = FSD,
+ .device_mode = DW_PCIE_EP_TYPE,
+};
+
static const struct samsung_pcie_pdata exynos_5433_pcie_rc_pdata = {
.dwc_ops = &exynos_dw_pcie_ops,
.pci_ops = &exynos_pci_ops,
@@ -485,6 +755,14 @@ static const struct of_device_id exynos_pcie_of_match[] = {
.compatible = "samsung,exynos5433-pcie",
.data = (void *) &exynos_5433_pcie_rc_pdata,
},
+ {
+ .compatible = "tesla,fsd-pcie",
+ .data = (void *) &fsd_hw3_pcie_rc_pdata,
+ },
+ {
+ .compatible = "tesla,fsd-pcie-ep",
+ .data = (void *) &fsd_hw3_pcie_ep_pdata,
+ },
{ },
};
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC
2025-08-11 15:46 ` [PATCH v3 11/12] PCI: exynos: Add support for Tesla " Shradha Todi
@ 2025-08-13 23:07 ` Bjorn Helgaas
2025-08-18 9:30 ` Shradha Todi
2025-08-30 3:54 ` Manivannan Sadhasivam
1 sibling, 1 reply; 34+ messages in thread
From: Bjorn Helgaas @ 2025-08-13 23:07 UTC (permalink / raw)
To: Shradha Todi
Cc: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy, mani, lpieralisi, kwilczynski, robh,
bhelgaas, jingoohan1, krzk+dt, conor+dt, alim.akhtar, vkoul,
kishon, arnd, m.szyprowski, jh80.chung, pankaj.dubey
On Mon, Aug 11, 2025 at 09:16:37PM +0530, Shradha Todi wrote:
> Add host and endpoint controller driver support for FSD SoC.
I think this might be easier if you added host mode first, then added
endpoint mode with a separate patch.
It's kind of unfortunate that the driver uses "ep" everywhere for
struct exynos_pcie pointers. It's going to be confusing because "ep"
is also commonly used for endpoint-related things, e.g., struct
dw_pcie_ep pointers. Maybe it's not worth changing; I dunno.
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
> drivers/pci/controller/dwc/pci-exynos.c | 278 ++++++++++++++++++++++++
> 1 file changed, 278 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
> index ef1f42236575..9aabfecdc147 100644
> --- a/drivers/pci/controller/dwc/pci-exynos.c
> +++ b/drivers/pci/controller/dwc/pci-exynos.c
> @@ -18,6 +18,8 @@
> #include <linux/platform_device.h>
> #include <linux/phy/phy.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
>
> @@ -49,15 +51,35 @@
> #define EXYNOS_PCIE_ELBI_SLV_ARMISC 0x120
> #define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE BIT(21)
>
> +#define FSD_IRQ2_STS 0x008
> +#define FSD_IRQ_MSI_ENABLE BIT(17)
> +#define FSD_IRQ2_EN 0x018
> +#define FSD_PCIE_APP_LTSSM_ENABLE 0x054
> +#define FSD_PCIE_LTSSM_ENABLE 0x1
> +#define FSD_PCIE_DEVICE_TYPE 0x080
> +#define FSD_DEVICE_TYPE_RC 0x4
> +#define FSD_DEVICE_TYPE_EP 0x0
> +#define FSD_PCIE_CXPL_DEBUG_00_31 0x2c8
> +
> /* to store different SoC variants of Samsung */
> enum samsung_pcie_variants {
> + FSD,
> EXYNOS_5433,
> };
>
> +/* Values to be written to SYSREG to view DBI space as CDM/DBI2/IATU/DMA */
> +enum fsd_pcie_addr_type {
> + ADDR_TYPE_DBI = 0x0,
> + ADDR_TYPE_DBI2 = 0x12,
> + ADDR_TYPE_ATU = 0x36,
> + ADDR_TYPE_DMA = 0x3f,
> +};
> +
> struct samsung_pcie_pdata {
> struct pci_ops *pci_ops;
> const struct dw_pcie_ops *dwc_ops;
> const struct dw_pcie_host_ops *host_ops;
> + const struct dw_pcie_ep_ops *ep_ops;
> const struct samsung_res_ops *res_ops;
> unsigned int soc_variant;
> enum dw_pcie_device_mode device_mode;
> @@ -67,6 +89,8 @@ struct exynos_pcie {
> struct dw_pcie pci;
> void __iomem *elbi_base;
> const struct samsung_pcie_pdata *pdata;
> + struct regmap *sysreg;
> + unsigned int sysreg_offset;
> struct clk_bulk_data *clks;
> struct phy *phy;
> struct regulator_bulk_data *supplies;
> @@ -334,11 +358,201 @@ static const struct dw_pcie_ops exynos_dw_pcie_ops = {
> .start_link = exynos_pcie_start_link,
> };
>
> +static void fsd_pcie_stop_link(struct dw_pcie *pci)
> +{
> + u32 val;
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> + val = readl(ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
> + val &= ~FSD_PCIE_LTSSM_ENABLE;
> + writel(val, ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
> +}
> +
> +static int fsd_pcie_start_link(struct dw_pcie *pci)
> +{
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
> + struct dw_pcie_ep *dw_ep = &pci->ep;
> +
> + if (dw_pcie_link_up(pci))
> + return 0;
> +
> + writel(FSD_PCIE_LTSSM_ENABLE, ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
> +
> + /* no need to wait for link in case of host as core files take care */
> + if (ep->pdata->device_mode == DW_PCIE_RC_TYPE)
> + return 0;
> +
> + /* check if the link is up or not in case of EP */
> + if (!dw_pcie_wait_for_link(pci)) {
> + dw_pcie_ep_linkup(dw_ep);
> + return 0;
> + }
> +
> + return -ETIMEDOUT;
> +}
> +
> +static irqreturn_t fsd_pcie_irq_handler(int irq, void *arg)
> +{
> + u32 val;
> + struct exynos_pcie *ep = arg;
> + struct dw_pcie *pci = &ep->pci;
> + struct dw_pcie_rp *pp = &pci->pp;
> +
> + val = readl(ep->elbi_base + FSD_IRQ2_STS);
> + if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
> + val &= FSD_IRQ_MSI_ENABLE;
> + writel(val, ep->elbi_base + FSD_IRQ2_STS);
This looks weird because FSD_IRQ_MSI_ENABLE sounds like an *enable*
bit, but here you're treating it as a *status* bit.
As far as I can tell, you set FSD_IRQ_MSI_ENABLE once at probe-time in
fsd_pcie_msi_init(), then you clear it here in an IRQ handler, and it
will never be set again. That seems wrong; am I missing something?
> + dw_handle_msi_irq(pp);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void fsd_pcie_msi_init(struct exynos_pcie *ep)
> +{
> + int val;
> +
> + val = readl(ep->elbi_base + FSD_IRQ2_EN);
> + val |= FSD_IRQ_MSI_ENABLE;
> + writel(val, ep->elbi_base + FSD_IRQ2_EN);
> +}
> +
> +static void __iomem *fsd_atu_setting(struct dw_pcie *pci, void __iomem *base)
The "setting" name suggests that this merely returns an address
without side effects, but in fact it actively *sets* the view.
In this case there's no locking around:
addr = fsd_atu_setting(pci, base);
dw_pcie_read(addr + reg, size, &val);
even though concurrent calls would cause issues, but I think that's OK
because we only get there via the driver, and I assume multiple DBI or
DBI2 accesses never happen because they're not used in asynchronous
paths like interrupt handlers.
But I think a name that hints at the fact that this does have side
effects would be helpful as a reminder in the callers that they must
not be used concurrently.
> +{
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> + if (base >= pci->atu_base) {
> + regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_ATU);
> + return (base - DEFAULT_DBI_ATU_OFFSET);
> + } else if (base == pci->dbi_base) {
> + regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
> + } else if (base == pci->dbi_base2) {
> + regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI2);
> + }
> +
> + return base;
> +}
> +
> +static u32 fsd_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size)
> +{
> + void __iomem *addr;
> + u32 val;
> +
> + addr = fsd_atu_setting(pci, base);
> + dw_pcie_read(addr + reg, size, &val);
> +
> + return val;
> +}
> +
> +static void fsd_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size, u32 val)
> +{
> + void __iomem *addr;
> +
> + addr = fsd_atu_setting(pci, base);
> + dw_pcie_write(addr + reg, size, val);
> +}
> +
> +static void fsd_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size, u32 val)
> +{
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> + fsd_atu_setting(pci, base);
> + dw_pcie_write(pci->dbi_base + reg, size, val);
> + regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
> +}
> +
> +static bool fsd_pcie_link_up(struct dw_pcie *pci)
> +{
> + u32 val;
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> + val = readl(ep->elbi_base + FSD_PCIE_CXPL_DEBUG_00_31);
> + val &= PORT_LOGIC_LTSSM_STATE_MASK;
> +
> + return (val == PORT_LOGIC_LTSSM_STATE_L0);
> +}
> +
> +static int fsd_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> + phy_init(ep->phy);
> + fsd_pcie_msi_init(ep);
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_host_ops fsd_pcie_host_ops = {
> + .init = fsd_pcie_host_init,
> +};
> +
> +static int fsd_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> + unsigned int type, u16 interrupt_num)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + switch (type) {
> + case PCI_IRQ_INTX:
> + return dw_pcie_ep_raise_intx_irq(ep, func_no);
> + case PCI_IRQ_MSIX:
> + dev_err(pci->dev, "EP does not support MSI-X\n");
> + return -EINVAL;
> + case PCI_IRQ_MSI:
> + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> + default:
> + dev_err(pci->dev, "UNKNOWN IRQ type\n");
> + }
> +
> + return 0;
> +}
> +
> +static const struct pci_epc_features fsd_pcie_epc_features = {
> + .linkup_notifier = false,
> + .msi_capable = true,
> + .msix_capable = false,
I think we should omit features we do *not* support instead of calling
them out explicitly, e.g., we don't need .linkup_notifier or
.msix_capable.
We've added them in the past, but they're unnecessary and they lead to
either pervasive changes (adding ".new_feature = false" to all
existing drivers when adding the feature) or inconsistency (new
drivers include ".new_feature = false" but existing drivers do not).
> +};
> +
> +static const struct pci_epc_features *fsd_pcie_get_features(struct dw_pcie_ep *ep)
> +{
> + return &fsd_pcie_epc_features;
> +}
> +
> +static const struct dw_pcie_ep_ops fsd_ep_ops = {
> + .raise_irq = fsd_pcie_raise_irq,
> + .get_features = fsd_pcie_get_features,
> +};
> +
> +static void fsd_set_device_mode(struct exynos_pcie *ep)
> +{
> + if (ep->pdata->device_mode == DW_PCIE_RC_TYPE)
> + writel(FSD_DEVICE_TYPE_RC, ep->elbi_base + FSD_PCIE_DEVICE_TYPE);
> + else
> + writel(FSD_DEVICE_TYPE_EP, ep->elbi_base + FSD_PCIE_DEVICE_TYPE);
> +}
> +
> +static const struct dw_pcie_ops fsd_dw_pcie_ops = {
> + .read_dbi = fsd_pcie_read_dbi,
> + .write_dbi = fsd_pcie_write_dbi,
> + .write_dbi2 = fsd_pcie_write_dbi2,
> + .start_link = fsd_pcie_start_link,
> + .stop_link = fsd_pcie_stop_link,
> + .link_up = fsd_pcie_link_up,
> +};
> +
> static const struct samsung_res_ops exynos_res_ops_data = {
> .init_regulator = exynos_init_regulator,
> .pcie_irq_handler = exynos_pcie_irq_handler,
> };
>
> +static const struct samsung_res_ops fsd_res_ops_data = {
> + .pcie_irq_handler = fsd_pcie_irq_handler,
> + .set_device_mode = fsd_set_device_mode,
> +};
> +
> static int exynos_pcie_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -361,6 +575,26 @@ static int exynos_pcie_probe(struct platform_device *pdev)
> if (IS_ERR(ep->phy))
> return PTR_ERR(ep->phy);
>
> + if (ep->pdata->soc_variant == FSD) {
> + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
> + if (ret)
> + return ret;
> +
> + ep->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
> + "samsung,syscon-pcie");
> + if (IS_ERR(ep->sysreg)) {
> + dev_err(dev, "sysreg regmap lookup failed.\n");
> + return PTR_ERR(ep->sysreg);
> + }
> +
> + ret = of_property_read_u32_index(dev->of_node, "samsung,syscon-pcie", 1,
> + &ep->sysreg_offset);
> + if (ret) {
> + dev_err(dev, "couldn't get the register offset for syscon!\n");
> + return ret;
> + }
> + }
This is a good example of a complicated set of things where I think
you should either add a SoC-specific function pointer to do this or
test a property, e.g., "DMA width", instead of testing for a specific
SoC.
> /* External Local Bus interface (ELBI) registers */
> ep->elbi_base = devm_platform_ioremap_resource_byname(pdev, "elbi");
> if (IS_ERR(ep->elbi_base))
> @@ -397,6 +631,22 @@ static int exynos_pcie_probe(struct platform_device *pdev)
> if (ret < 0)
> goto fail_phy_init;
>
> + break;
> + case DW_PCIE_EP_TYPE:
> + phy_init(ep->phy);
> +
> + ep->pci.ep.ops = pdata->ep_ops;
> +
> + ret = dw_pcie_ep_init(&ep->pci.ep);
> + if (ret < 0)
> + goto fail_phy_init;
> +
> + ret = dw_pcie_ep_init_registers(&ep->pci.ep);
> + if (ret)
> + goto fail_phy_init;
> +
> + pci_epc_init_notify(ep->pci.ep.epc);
> +
> break;
> default:
> dev_err(dev, "invalid device type\n");
> @@ -436,6 +686,9 @@ static int exynos_pcie_suspend_noirq(struct device *dev)
> if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
> return 0;
>
> + if (ep->pdata->dwc_ops->stop_link)
> + ep->pdata->dwc_ops->stop_link(pci);
> +
> if (ep->pdata->soc_variant == EXYNOS_5433)
> exynos_pcie_assert_core_reset(ep);
> phy_power_off(ep->phy);
> @@ -471,6 +724,23 @@ static const struct dev_pm_ops exynos_pcie_pm_ops = {
> exynos_pcie_resume_noirq)
> };
>
> +
> +static const struct samsung_pcie_pdata fsd_hw3_pcie_rc_pdata = {
> + .dwc_ops = &fsd_dw_pcie_ops,
> + .host_ops = &fsd_pcie_host_ops,
> + .res_ops = &fsd_res_ops_data,
> + .soc_variant = FSD,
> + .device_mode = DW_PCIE_RC_TYPE,
> +};
> +
> +static const struct samsung_pcie_pdata fsd_hw3_pcie_ep_pdata = {
> + .dwc_ops = &fsd_dw_pcie_ops,
> + .ep_ops = &fsd_ep_ops,
> + .res_ops = &fsd_res_ops_data,
> + .soc_variant = FSD,
> + .device_mode = DW_PCIE_EP_TYPE,
> +};
> +
> static const struct samsung_pcie_pdata exynos_5433_pcie_rc_pdata = {
> .dwc_ops = &exynos_dw_pcie_ops,
> .pci_ops = &exynos_pci_ops,
> @@ -485,6 +755,14 @@ static const struct of_device_id exynos_pcie_of_match[] = {
> .compatible = "samsung,exynos5433-pcie",
> .data = (void *) &exynos_5433_pcie_rc_pdata,
> },
> + {
> + .compatible = "tesla,fsd-pcie",
> + .data = (void *) &fsd_hw3_pcie_rc_pdata,
> + },
> + {
> + .compatible = "tesla,fsd-pcie-ep",
> + .data = (void *) &fsd_hw3_pcie_ep_pdata,
> + },
> { },
> };
>
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 34+ messages in thread* RE: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC
2025-08-13 23:07 ` Bjorn Helgaas
@ 2025-08-18 9:30 ` Shradha Todi
2025-08-18 18:25 ` Bjorn Helgaas
0 siblings, 1 reply; 34+ messages in thread
From: Shradha Todi @ 2025-08-18 9:30 UTC (permalink / raw)
To: 'Bjorn Helgaas'
Cc: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy, mani, lpieralisi, kwilczynski, robh,
bhelgaas, jingoohan1, krzk+dt, conor+dt, alim.akhtar, vkoul,
kishon, arnd, m.szyprowski, jh80.chung, pankaj.dubey
> On Mon, Aug 11, 2025 at 09:16:37PM +0530, Shradha Todi wrote:
> > Add host and endpoint controller driver support for FSD SoC.
>
> I think this might be easier if you added host mode first, then added
> endpoint mode with a separate patch.
>
Will do.
> It's kind of unfortunate that the driver uses "ep" everywhere for
> struct exynos_pcie pointers. It's going to be confusing because "ep"
> is also commonly used for endpoint-related things, e.g., struct
> dw_pcie_ep pointers. Maybe it's not worth changing; I dunno.
>
I did try to rename the structure and the pointers
(https://lore.kernel.org/all/20230214121333.1837-9-shradha.t@samsung.com/)
But the intention was different back then and so the idea was rejected.
I could add a patch to only rename the pointers to something less
confusing like "exy_pci"
> > +static irqreturn_t fsd_pcie_irq_handler(int irq, void *arg)
> > +{
> > + u32 val;
> > + struct exynos_pcie *ep = arg;
> > + struct dw_pcie *pci = &ep->pci;
> > + struct dw_pcie_rp *pp = &pci->pp;
> > +
> > + val = readl(ep->elbi_base + FSD_IRQ2_STS);
> > + if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
> > + val &= FSD_IRQ_MSI_ENABLE;
> > + writel(val, ep->elbi_base + FSD_IRQ2_STS);
>
> This looks weird because FSD_IRQ_MSI_ENABLE sounds like an *enable*
> bit, but here you're treating it as a *status* bit.
>
> As far as I can tell, you set FSD_IRQ_MSI_ENABLE once at probe-time in
> fsd_pcie_msi_init(), then you clear it here in an IRQ handler, and it
> will never be set again. That seems wrong; am I missing something?
>
Actually the status IRQ and enable IRQ registers are different offsets
but the bit position for MSI remains same in both cases so I just reused
the macro. But I understand that it's confusing so I will add another
macro for FSD_IRQ_MSI_STATUS or just rename the macro to
FSD_IRQ_MSI to re-use.
> > + dw_handle_msi_irq(pp);
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static void fsd_pcie_msi_init(struct exynos_pcie *ep)
> > +{
> > + int val;
> > +
> > + val = readl(ep->elbi_base + FSD_IRQ2_EN);
> > + val |= FSD_IRQ_MSI_ENABLE;
> > + writel(val, ep->elbi_base + FSD_IRQ2_EN);
> > +}
> > +
> > +static void __iomem *fsd_atu_setting(struct dw_pcie *pci, void __iomem *base)
>
> The "setting" name suggests that this merely returns an address
> without side effects, but in fact it actively *sets* the view.
>
> In this case there's no locking around:
>
> addr = fsd_atu_setting(pci, base);
> dw_pcie_read(addr + reg, size, &val);
>
> even though concurrent calls would cause issues, but I think that's OK
> because we only get there via the driver, and I assume multiple DBI or
> DBI2 accesses never happen because they're not used in asynchronous
> paths like interrupt handlers.
>
Yes, there is no concurrent access to this function and hence I have
not added locking mechanism.
> But I think a name that hints at the fact that this does have side
> effects would be helpful as a reminder in the callers that they must
> not be used concurrently.
>
Sure, I will change the name and also add comment as a reminder.
> > +static const struct pci_epc_features fsd_pcie_epc_features = {
> > + .linkup_notifier = false,
> > + .msi_capable = true,
> > + .msix_capable = false,
>
> I think we should omit features we do *not* support instead of calling
> them out explicitly, e.g., we don't need .linkup_notifier or
> .msix_capable.
>
> We've added them in the past, but they're unnecessary and they lead to
> either pervasive changes (adding ".new_feature = false" to all
> existing drivers when adding the feature) or inconsistency (new
> drivers include ".new_feature = false" but existing drivers do not).
>
Will remove
> > + if (ep->pdata->soc_variant == FSD) {
> > + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
> > + if (ret)
> > + return ret;
> > +
> > + ep->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
> > + "samsung,syscon-pcie");
> > + if (IS_ERR(ep->sysreg)) {
> > + dev_err(dev, "sysreg regmap lookup failed.\n");
> > + return PTR_ERR(ep->sysreg);
> > + }
> > +
> > + ret = of_property_read_u32_index(dev->of_node, "samsung,syscon-pcie", 1,
> > + &ep->sysreg_offset);
> > + if (ret) {
> > + dev_err(dev, "couldn't get the register offset for syscon!\n");
> > + return ret;
> > + }
> > + }
>
> This is a good example of a complicated set of things where I think
> you should either add a SoC-specific function pointer to do this or
> test a property, e.g., "DMA width", instead of testing for a specific
> SoC.
>
Got your point and it makes sense. In future, other drivers could also
want to set DMA width, etc. Will make properties to replace soc_variant:
- DMA_width
- has_syscon
- function pointer to assert_core_reset and deassert_core_reset
Any suggestions or is this approach okay?
-Shradha
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC
2025-08-18 9:30 ` Shradha Todi
@ 2025-08-18 18:25 ` Bjorn Helgaas
2025-08-19 6:34 ` Krzysztof Kozlowski
2025-08-19 11:39 ` Shradha Todi
0 siblings, 2 replies; 34+ messages in thread
From: Bjorn Helgaas @ 2025-08-18 18:25 UTC (permalink / raw)
To: Shradha Todi, Krzysztof Kozlowski
Cc: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy, mani, lpieralisi, kwilczynski, robh,
bhelgaas, jingoohan1, krzk+dt, conor+dt, alim.akhtar, vkoul,
kishon, arnd, m.szyprowski, jh80.chung, pankaj.dubey
[+to Krzysztof]
On Mon, Aug 18, 2025 at 03:00:00PM +0530, Shradha Todi wrote:
> > On Mon, Aug 11, 2025 at 09:16:37PM +0530, Shradha Todi wrote:
> > > Add host and endpoint controller driver support for FSD SoC.
> > It's kind of unfortunate that the driver uses "ep" everywhere for
> > struct exynos_pcie pointers. It's going to be confusing because "ep"
> > is also commonly used for endpoint-related things, e.g., struct
> > dw_pcie_ep pointers. Maybe it's not worth changing; I dunno.
>
> I did try to rename the structure and the pointers
> (https://lore.kernel.org/all/20230214121333.1837-9-shradha.t@samsung.com/)
> But the intention was different back then and so the idea was rejected.
> I could add a patch to only rename the pointers to something less
> confusing like "exy_pci"
The patch you mention did several renames:
s/to_exynos_pcie/to_samsung_pcie/
s/struct exynos_pcie/struct samsung_pcie/
s/struct exynos_pcie *ep/struct samsung_pcie *sp/
I'm only concerned about the confusion of "ep" being used both for
"struct exynos_pcie *" and for "struct dw_pcie_ep *".
It would still be sort of an annoying patch to do something like this:
s/struct exynos_pcie *ep/struct exynos_pcie *pcie/
But 'git grep "struct .*_pcie \*.*=" drivers/pci/controller/' says
using "pcie" in this way is quite common, so maybe it would be worth
doing.
What do you think, Krzysztof?
> > > +static irqreturn_t fsd_pcie_irq_handler(int irq, void *arg)
> > > +{
> > > + u32 val;
> > > + struct exynos_pcie *ep = arg;
> > > + struct dw_pcie *pci = &ep->pci;
> > > + struct dw_pcie_rp *pp = &pci->pp;
> > > +
> > > + val = readl(ep->elbi_base + FSD_IRQ2_STS);
> > > + if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
> > > + val &= FSD_IRQ_MSI_ENABLE;
> > > + writel(val, ep->elbi_base + FSD_IRQ2_STS);
> >
> > This looks weird because FSD_IRQ_MSI_ENABLE sounds like an *enable*
> > bit, but here you're treating it as a *status* bit.
> >
> > As far as I can tell, you set FSD_IRQ_MSI_ENABLE once at probe-time in
> > fsd_pcie_msi_init(), then you clear it here in an IRQ handler, and it
> > will never be set again. That seems wrong; am I missing something?
>
> Actually the status IRQ and enable IRQ registers are different offsets
> but the bit position for MSI remains same in both cases so I just reused
> the macro.
Ah, that's what I missed, thanks! At probe-time, fsd_pcie_msi_init()
enables it in FSD_IRQ2_EN. Here you clear it in FSD_IRQ2_STS.
> But I understand that it's confusing so I will add another
> macro for FSD_IRQ_MSI_STATUS or just rename the macro to
> FSD_IRQ_MSI to re-use.
Using the same name just because a similar bit happens to be at the
same position in two different registers is definitely confusing. I
think it will be better to have two macros, one for FSD_IRQ2_STS and
another for FSD_IRQ2_EN, e.g.,
#define FSD_IRQ2_STS 0x008
#define FSD_IRQ2_STS_MSI BIT(17)
#define FSD_IRQ2_EN 0x018
#define FSD_IRQ2_EN_MSI BIT(17)
Another question about the test:
if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
This assumes there are no other bits in FSD_IRQ2_STS that could be
set. I would have expected a test like this:
if (val & FSD_IRQ_MSI_ENABLE) {
Is there a reason to restrict it to the case when *only*
FSD_IRQ_MSI_ENABLE is set?
Bjorn
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC
2025-08-18 18:25 ` Bjorn Helgaas
@ 2025-08-19 6:34 ` Krzysztof Kozlowski
2025-08-19 11:18 ` Shradha Todi
2025-08-19 11:39 ` Shradha Todi
1 sibling, 1 reply; 34+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-19 6:34 UTC (permalink / raw)
To: Bjorn Helgaas, Shradha Todi, Krzysztof Kozlowski
Cc: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy, mani, lpieralisi, kwilczynski, robh,
bhelgaas, jingoohan1, krzk+dt, conor+dt, alim.akhtar, vkoul,
kishon, arnd, m.szyprowski, jh80.chung, pankaj.dubey
On 18/08/2025 20:25, Bjorn Helgaas wrote:
> [+to Krzysztof]
>
> On Mon, Aug 18, 2025 at 03:00:00PM +0530, Shradha Todi wrote:
>>> On Mon, Aug 11, 2025 at 09:16:37PM +0530, Shradha Todi wrote:
>>>> Add host and endpoint controller driver support for FSD SoC.
>
>>> It's kind of unfortunate that the driver uses "ep" everywhere for
>>> struct exynos_pcie pointers. It's going to be confusing because "ep"
>>> is also commonly used for endpoint-related things, e.g., struct
>>> dw_pcie_ep pointers. Maybe it's not worth changing; I dunno.
>>
>> I did try to rename the structure and the pointers
>> (https://lore.kernel.org/all/20230214121333.1837-9-shradha.t@samsung.com/)
>> But the intention was different back then and so the idea was rejected.
>> I could add a patch to only rename the pointers to something less
>> confusing like "exy_pci"
>
> The patch you mention did several renames:
>
> s/to_exynos_pcie/to_samsung_pcie/
> s/struct exynos_pcie/struct samsung_pcie/
> s/struct exynos_pcie *ep/struct samsung_pcie *sp/
>
> I'm only concerned about the confusion of "ep" being used both for
> "struct exynos_pcie *" and for "struct dw_pcie_ep *".
>
> It would still be sort of an annoying patch to do something like this:
>
> s/struct exynos_pcie *ep/struct exynos_pcie *pcie/
>
> But 'git grep "struct .*_pcie \*.*=" drivers/pci/controller/' says
> using "pcie" in this way is quite common, so maybe it would be worth
> doing.
>
> What do you think, Krzysztof?
I think you want other Krzysztof, but nevertheless, the reasoning there
"Changing it to samsung_pcie for making it
generic."
is wrong. The naming of these structures do not matter, they are not
less generic. This is rather churn, which will affect backporting for
ZERO readability increase. Why zero? Because calling all this "exynos"
is the same as calling all this "samsung". It just does not matter.
However s/ep/pcie/ in variable name makes sense if that's more common.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC
2025-08-19 6:34 ` Krzysztof Kozlowski
@ 2025-08-19 11:18 ` Shradha Todi
0 siblings, 0 replies; 34+ messages in thread
From: Shradha Todi @ 2025-08-19 11:18 UTC (permalink / raw)
To: 'Krzysztof Kozlowski', 'Bjorn Helgaas',
'Krzysztof Kozlowski'
Cc: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy, mani, lpieralisi, kwilczynski, robh,
bhelgaas, jingoohan1, krzk+dt, conor+dt, alim.akhtar, vkoul,
kishon, arnd, m.szyprowski, jh80.chung, pankaj.dubey
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 19 August 2025 12:04
> To: Bjorn Helgaas <helgaas@kernel.org>; Shradha Todi <shradha.t@samsung.com>; Krzysztof
> Kozlowski <krzysztof.kozlowski@linaro.org>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-samsung-soc@vger.kernel.org; linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org;
> mani@kernel.org; lpieralisi@kernel.org; kwilczynski@kernel.org; robh@kernel.org;
> bhelgaas@google.com; jingoohan1@gmail.com; krzk+dt@kernel.org; conor+dt@kernel.org;
> alim.akhtar@samsung.com; vkoul@kernel.org; kishon@kernel.org; arnd@arndb.de;
> m.szyprowski@samsung.com; jh80.chung@samsung.com; pankaj.dubey@samsung.com
> Subject: Re: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC
>
> On 18/08/2025 20:25, Bjorn Helgaas wrote:
> > [+to Krzysztof]
> >
> > On Mon, Aug 18, 2025 at 03:00:00PM +0530, Shradha Todi wrote:
> >>> On Mon, Aug 11, 2025 at 09:16:37PM +0530, Shradha Todi wrote:
> >>>> Add host and endpoint controller driver support for FSD SoC.
> >
> >>> It's kind of unfortunate that the driver uses "ep" everywhere for
> >>> struct exynos_pcie pointers. It's going to be confusing because "ep"
> >>> is also commonly used for endpoint-related things, e.g., struct
> >>> dw_pcie_ep pointers. Maybe it's not worth changing; I dunno.
> >>
> >> I did try to rename the structure and the pointers
> >> (https://lore.kernel.org/all/20230214121333.1837-9-shradha.t@samsung.com/)
> >> But the intention was different back then and so the idea was rejected.
> >> I could add a patch to only rename the pointers to something less
> >> confusing like "exy_pci"
> >
> > The patch you mention did several renames:
> >
> > s/to_exynos_pcie/to_samsung_pcie/
> > s/struct exynos_pcie/struct samsung_pcie/
> > s/struct exynos_pcie *ep/struct samsung_pcie *sp/
> >
> > I'm only concerned about the confusion of "ep" being used both for
> > "struct exynos_pcie *" and for "struct dw_pcie_ep *".
> >
> > It would still be sort of an annoying patch to do something like this:
> >
> > s/struct exynos_pcie *ep/struct exynos_pcie *pcie/
> >
> > But 'git grep "struct .*_pcie \*.*=" drivers/pci/controller/' says
> > using "pcie" in this way is quite common, so maybe it would be worth
> > doing.
> >
> > What do you think, Krzysztof?
>
> I think you want other Krzysztof, but nevertheless, the reasoning there
> "Changing it to samsung_pcie for making it
> generic."
> is wrong. The naming of these structures do not matter, they are not
> less generic. This is rather churn, which will affect backporting for
> ZERO readability increase. Why zero? Because calling all this "exynos"
> is the same as calling all this "samsung". It just does not matter.
>
> However s/ep/pcie/ in variable name makes sense if that's more common.
>
I will add a patch in the series to do that.
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC
2025-08-18 18:25 ` Bjorn Helgaas
2025-08-19 6:34 ` Krzysztof Kozlowski
@ 2025-08-19 11:39 ` Shradha Todi
2025-08-19 15:07 ` Bjorn Helgaas
1 sibling, 1 reply; 34+ messages in thread
From: Shradha Todi @ 2025-08-19 11:39 UTC (permalink / raw)
To: 'Bjorn Helgaas', 'Krzysztof Kozlowski'
Cc: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy, mani, lpieralisi, kwilczynski, robh,
bhelgaas, jingoohan1, krzk+dt, conor+dt, alim.akhtar, vkoul,
kishon, arnd, m.szyprowski, jh80.chung, pankaj.dubey
> > > > +static irqreturn_t fsd_pcie_irq_handler(int irq, void *arg)
> > > > +{
> > > > + u32 val;
> > > > + struct exynos_pcie *ep = arg;
> > > > + struct dw_pcie *pci = &ep->pci;
> > > > + struct dw_pcie_rp *pp = &pci->pp;
> > > > +
> > > > + val = readl(ep->elbi_base + FSD_IRQ2_STS);
> > > > + if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
> > > > + val &= FSD_IRQ_MSI_ENABLE;
> > > > + writel(val, ep->elbi_base + FSD_IRQ2_STS);
> > >
> > > This looks weird because FSD_IRQ_MSI_ENABLE sounds like an *enable*
> > > bit, but here you're treating it as a *status* bit.
> > >
> > > As far as I can tell, you set FSD_IRQ_MSI_ENABLE once at probe-time in
> > > fsd_pcie_msi_init(), then you clear it here in an IRQ handler, and it
> > > will never be set again. That seems wrong; am I missing something?
> >
> > Actually the status IRQ and enable IRQ registers are different offsets
> > but the bit position for MSI remains same in both cases so I just reused
> > the macro.
>
> Ah, that's what I missed, thanks! At probe-time, fsd_pcie_msi_init()
> enables it in FSD_IRQ2_EN. Here you clear it in FSD_IRQ2_STS.
>
> > But I understand that it's confusing so I will add another
> > macro for FSD_IRQ_MSI_STATUS or just rename the macro to
> > FSD_IRQ_MSI to re-use.
>
> Using the same name just because a similar bit happens to be at the
> same position in two different registers is definitely confusing. I
> think it will be better to have two macros, one for FSD_IRQ2_STS and
> another for FSD_IRQ2_EN, e.g.,
>
> #define FSD_IRQ2_STS 0x008
> #define FSD_IRQ2_STS_MSI BIT(17)
> #define FSD_IRQ2_EN 0x018
> #define FSD_IRQ2_EN_MSI BIT(17)
>
> Another question about the test:
>
> if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
>
> This assumes there are no other bits in FSD_IRQ2_STS that could be
> set. I would have expected a test like this:
>
> if (val & FSD_IRQ_MSI_ENABLE) {
>
Thanks for pointing this out. FSD_IRQ_MSI_ENABLE is a single-bit, so there
is no functional difference in the two statements. I didn't have a specific
reason for using "== FSD_IRQ_MSI_ENABLE".
But I see that "val & FSD_IRQ_MSI_ENABLE" would have been the more
standard way to write this. I will update this for clarity.
> Is there a reason to restrict it to the case when *only*
> FSD_IRQ_MSI_ENABLE is set?
>
> Bjorn
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC
2025-08-19 11:39 ` Shradha Todi
@ 2025-08-19 15:07 ` Bjorn Helgaas
0 siblings, 0 replies; 34+ messages in thread
From: Bjorn Helgaas @ 2025-08-19 15:07 UTC (permalink / raw)
To: Shradha Todi
Cc: 'Krzysztof Kozlowski', linux-pci, devicetree,
linux-arm-kernel, linux-samsung-soc, linux-kernel, linux-phy,
mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey
On Tue, Aug 19, 2025 at 05:09:34PM +0530, Shradha Todi wrote:
> ...
> > Another question about the test:
> >
> > if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
> >
> > This assumes there are no other bits in FSD_IRQ2_STS that could be
> > set. I would have expected a test like this:
> >
> > if (val & FSD_IRQ_MSI_ENABLE) {
>
> Thanks for pointing this out. FSD_IRQ_MSI_ENABLE is a single-bit, so there
> is no functional difference in the two statements. I didn't have a specific
> reason for using "== FSD_IRQ_MSI_ENABLE".
> But I see that "val & FSD_IRQ_MSI_ENABLE" would have been the more
> standard way to write this. I will update this for clarity.
Oof, sorry, I don't know what I was thinking. You're right, it's OK
as is. But "val & FSD_IRQ_MSI_ENABLE" *is* shorter and more
idiomatic, so I think preferable anyway.
Bjorn
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC
2025-08-11 15:46 ` [PATCH v3 11/12] PCI: exynos: Add support for Tesla " Shradha Todi
2025-08-13 23:07 ` Bjorn Helgaas
@ 2025-08-30 3:54 ` Manivannan Sadhasivam
1 sibling, 0 replies; 34+ messages in thread
From: Manivannan Sadhasivam @ 2025-08-30 3:54 UTC (permalink / raw)
To: Shradha Todi
Cc: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy, lpieralisi, kwilczynski, robh, bhelgaas,
jingoohan1, krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd,
m.szyprowski, jh80.chung, pankaj.dubey
On Mon, Aug 11, 2025 at 09:16:37PM GMT, Shradha Todi wrote:
> Add host and endpoint controller driver support for FSD SoC.
>
Again, more info about the controller required.
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
> drivers/pci/controller/dwc/pci-exynos.c | 278 ++++++++++++++++++++++++
> 1 file changed, 278 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
> index ef1f42236575..9aabfecdc147 100644
> --- a/drivers/pci/controller/dwc/pci-exynos.c
> +++ b/drivers/pci/controller/dwc/pci-exynos.c
> @@ -18,6 +18,8 @@
> #include <linux/platform_device.h>
> #include <linux/phy/phy.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
>
> @@ -49,15 +51,35 @@
> #define EXYNOS_PCIE_ELBI_SLV_ARMISC 0x120
> #define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE BIT(21)
>
> +#define FSD_IRQ2_STS 0x008
> +#define FSD_IRQ_MSI_ENABLE BIT(17)
> +#define FSD_IRQ2_EN 0x018
> +#define FSD_PCIE_APP_LTSSM_ENABLE 0x054
> +#define FSD_PCIE_LTSSM_ENABLE 0x1
> +#define FSD_PCIE_DEVICE_TYPE 0x080
> +#define FSD_DEVICE_TYPE_RC 0x4
> +#define FSD_DEVICE_TYPE_EP 0x0
> +#define FSD_PCIE_CXPL_DEBUG_00_31 0x2c8
> +
> /* to store different SoC variants of Samsung */
> enum samsung_pcie_variants {
> + FSD,
> EXYNOS_5433,
I hope you are going to drop these variants and use the properties for specific
functionality as suggested by Bjorn.
> };
>
> +/* Values to be written to SYSREG to view DBI space as CDM/DBI2/IATU/DMA */
> +enum fsd_pcie_addr_type {
> + ADDR_TYPE_DBI = 0x0,
> + ADDR_TYPE_DBI2 = 0x12,
> + ADDR_TYPE_ATU = 0x36,
> + ADDR_TYPE_DMA = 0x3f,
> +};
> +
> struct samsung_pcie_pdata {
> struct pci_ops *pci_ops;
> const struct dw_pcie_ops *dwc_ops;
> const struct dw_pcie_host_ops *host_ops;
> + const struct dw_pcie_ep_ops *ep_ops;
> const struct samsung_res_ops *res_ops;
> unsigned int soc_variant;
> enum dw_pcie_device_mode device_mode;
> @@ -67,6 +89,8 @@ struct exynos_pcie {
> struct dw_pcie pci;
> void __iomem *elbi_base;
> const struct samsung_pcie_pdata *pdata;
> + struct regmap *sysreg;
> + unsigned int sysreg_offset;
> struct clk_bulk_data *clks;
> struct phy *phy;
> struct regulator_bulk_data *supplies;
> @@ -334,11 +358,201 @@ static const struct dw_pcie_ops exynos_dw_pcie_ops = {
> .start_link = exynos_pcie_start_link,
> };
>
> +static void fsd_pcie_stop_link(struct dw_pcie *pci)
> +{
> + u32 val;
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
Please use reverse Xmas order for local variables.
> +
> + val = readl(ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
> + val &= ~FSD_PCIE_LTSSM_ENABLE;
> + writel(val, ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
> +}
> +
> +static int fsd_pcie_start_link(struct dw_pcie *pci)
> +{
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
> + struct dw_pcie_ep *dw_ep = &pci->ep;
> +
> + if (dw_pcie_link_up(pci))
> + return 0;
> +
> + writel(FSD_PCIE_LTSSM_ENABLE, ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
> +
> + /* no need to wait for link in case of host as core files take care */
What do you mean by 'core files'? DWC core driver?
> + if (ep->pdata->device_mode == DW_PCIE_RC_TYPE)
> + return 0;
> +
> + /* check if the link is up or not in case of EP */
> + if (!dw_pcie_wait_for_link(pci)) {
I presume there is no way to detect link up using the SPI interrupt?
> + dw_pcie_ep_linkup(dw_ep);
> + return 0;
> + }
> +
> + return -ETIMEDOUT;
> +}
> +
> +static irqreturn_t fsd_pcie_irq_handler(int irq, void *arg)
> +{
> + u32 val;
> + struct exynos_pcie *ep = arg;
> + struct dw_pcie *pci = &ep->pci;
> + struct dw_pcie_rp *pp = &pci->pp;
> +
> + val = readl(ep->elbi_base + FSD_IRQ2_STS);
> + if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
> + val &= FSD_IRQ_MSI_ENABLE;
> + writel(val, ep->elbi_base + FSD_IRQ2_STS);
> + dw_handle_msi_irq(pp);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void fsd_pcie_msi_init(struct exynos_pcie *ep)
> +{
> + int val;
> +
> + val = readl(ep->elbi_base + FSD_IRQ2_EN);
> + val |= FSD_IRQ_MSI_ENABLE;
> + writel(val, ep->elbi_base + FSD_IRQ2_EN);
Could you please explain how MSI handling works? I don't understand it.
> +}
> +
> +static void __iomem *fsd_atu_setting(struct dw_pcie *pci, void __iomem *base)
> +{
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> + if (base >= pci->atu_base) {
> + regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_ATU);
> + return (base - DEFAULT_DBI_ATU_OFFSET);
> + } else if (base == pci->dbi_base) {
> + regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
> + } else if (base == pci->dbi_base2) {
> + regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI2);
> + }
> +
> + return base;
> +}
> +
> +static u32 fsd_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size)
> +{
> + void __iomem *addr;
> + u32 val;
> +
> + addr = fsd_atu_setting(pci, base);
The returned 'addr' seems to be static. Why can't you just store it during probe
and use the 'addr' directly?
> + dw_pcie_read(addr + reg, size, &val);
> +
> + return val;
> +}
> +
> +static void fsd_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size, u32 val)
> +{
> + void __iomem *addr;
> +
> + addr = fsd_atu_setting(pci, base);
> + dw_pcie_write(addr + reg, size, val);
> +}
> +
> +static void fsd_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size, u32 val)
> +{
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> + fsd_atu_setting(pci, base);
> + dw_pcie_write(pci->dbi_base + reg, size, val);
> + regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
> +}
> +
> +static bool fsd_pcie_link_up(struct dw_pcie *pci)
> +{
> + u32 val;
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> + val = readl(ep->elbi_base + FSD_PCIE_CXPL_DEBUG_00_31);
> + val &= PORT_LOGIC_LTSSM_STATE_MASK;
> +
> + return (val == PORT_LOGIC_LTSSM_STATE_L0);
> +}
> +
> +static int fsd_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> + phy_init(ep->phy);
> + fsd_pcie_msi_init(ep);
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_host_ops fsd_pcie_host_ops = {
> + .init = fsd_pcie_host_init,
> +};
> +
> +static int fsd_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> + unsigned int type, u16 interrupt_num)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + switch (type) {
> + case PCI_IRQ_INTX:
> + return dw_pcie_ep_raise_intx_irq(ep, func_no);
> + case PCI_IRQ_MSIX:
> + dev_err(pci->dev, "EP does not support MSI-X\n");
> + return -EINVAL;
> + case PCI_IRQ_MSI:
> + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> + default:
> + dev_err(pci->dev, "UNKNOWN IRQ type\n");
Do you want to return success below even for this error?
> + }
> +
> + return 0;
this should be -EOPNOTSUPP.
> +}
> +
> +static const struct pci_epc_features fsd_pcie_epc_features = {
> + .linkup_notifier = false,
Default value is 'false'. So no need to initialize it agian.
> + .msi_capable = true,
> + .msix_capable = false,
Here also.
> +};
> +
> +static const struct pci_epc_features *fsd_pcie_get_features(struct dw_pcie_ep *ep)
> +{
> + return &fsd_pcie_epc_features;
> +}
> +
> +static const struct dw_pcie_ep_ops fsd_ep_ops = {
> + .raise_irq = fsd_pcie_raise_irq,
> + .get_features = fsd_pcie_get_features,
> +};
> +
> +static void fsd_set_device_mode(struct exynos_pcie *ep)
> +{
> + if (ep->pdata->device_mode == DW_PCIE_RC_TYPE)
> + writel(FSD_DEVICE_TYPE_RC, ep->elbi_base + FSD_PCIE_DEVICE_TYPE);
> + else
> + writel(FSD_DEVICE_TYPE_EP, ep->elbi_base + FSD_PCIE_DEVICE_TYPE);
> +}
> +
> +static const struct dw_pcie_ops fsd_dw_pcie_ops = {
> + .read_dbi = fsd_pcie_read_dbi,
> + .write_dbi = fsd_pcie_write_dbi,
> + .write_dbi2 = fsd_pcie_write_dbi2,
> + .start_link = fsd_pcie_start_link,
> + .stop_link = fsd_pcie_stop_link,
> + .link_up = fsd_pcie_link_up,
> +};
> +
> static const struct samsung_res_ops exynos_res_ops_data = {
> .init_regulator = exynos_init_regulator,
> .pcie_irq_handler = exynos_pcie_irq_handler,
> };
>
> +static const struct samsung_res_ops fsd_res_ops_data = {
> + .pcie_irq_handler = fsd_pcie_irq_handler,
> + .set_device_mode = fsd_set_device_mode,
> +};
> +
> static int exynos_pcie_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -361,6 +575,26 @@ static int exynos_pcie_probe(struct platform_device *pdev)
> if (IS_ERR(ep->phy))
> return PTR_ERR(ep->phy);
>
> + if (ep->pdata->soc_variant == FSD) {
> + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
> + if (ret)
> + return ret;
> +
> + ep->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
> + "samsung,syscon-pcie");
> + if (IS_ERR(ep->sysreg)) {
> + dev_err(dev, "sysreg regmap lookup failed.\n");
Use dev_err_probe() here and below.
> + return PTR_ERR(ep->sysreg);
> + }
> +
> + ret = of_property_read_u32_index(dev->of_node, "samsung,syscon-pcie", 1,
> + &ep->sysreg_offset);
> + if (ret) {
> + dev_err(dev, "couldn't get the register offset for syscon!\n");
> + return ret;
> + }
> + }
> +
> /* External Local Bus interface (ELBI) registers */
> ep->elbi_base = devm_platform_ioremap_resource_byname(pdev, "elbi");
> if (IS_ERR(ep->elbi_base))
> @@ -397,6 +631,22 @@ static int exynos_pcie_probe(struct platform_device *pdev)
> if (ret < 0)
> goto fail_phy_init;
>
> + break;
> + case DW_PCIE_EP_TYPE:
> + phy_init(ep->phy);
> +
> + ep->pci.ep.ops = pdata->ep_ops;
> +
> + ret = dw_pcie_ep_init(&ep->pci.ep);
> + if (ret < 0)
> + goto fail_phy_init;
> +
> + ret = dw_pcie_ep_init_registers(&ep->pci.ep);
> + if (ret)
> + goto fail_phy_init;
> +
> + pci_epc_init_notify(ep->pci.ep.epc);
So the platform doesn't support PERST# and generates its own refclk?
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 12/12] arm64: dts: fsd: Add PCIe support for Tesla FSD SoC
2025-08-11 15:46 ` [PATCH v3 00/12] Add PCIe support for Tesla FSD SoC Shradha Todi
` (10 preceding siblings ...)
2025-08-11 15:46 ` [PATCH v3 11/12] PCI: exynos: Add support for Tesla " Shradha Todi
@ 2025-08-11 15:46 ` Shradha Todi
2025-08-12 6:43 ` Krzysztof Kozlowski
2025-08-30 3:58 ` Manivannan Sadhasivam
11 siblings, 2 replies; 34+ messages in thread
From: Shradha Todi @ 2025-08-11 15:46 UTC (permalink / raw)
To: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey, Shradha Todi
Add the support for PCIe controller driver and phy driver for Tesla FSD.
It includes support for both RC and EP.
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
arch/arm64/boot/dts/tesla/fsd-evb.dts | 34 +++++
arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 65 +++++++++
arch/arm64/boot/dts/tesla/fsd.dtsi | 147 +++++++++++++++++++++
3 files changed, 246 insertions(+)
diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
index 9ff22e1c8723..1b63c5d72d19 100644
--- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
+++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
@@ -130,3 +130,37 @@ &serial_0 {
&ufs {
status = "okay";
};
+
+&pcierc2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
+ <&pcie0_slot1>;
+};
+
+&pcieep2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
+ <&pcie0_slot1>;
+};
+
+&pcierc0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
+ <&pcie0_slot0>;
+};
+
+&pcieep0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
+ <&pcie0_slot0>;
+};
+
+&pcierc1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
+};
+
+&pcieep1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
+};
diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
index 6f4658f57453..fa99aa4b9906 100644
--- a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
@@ -120,6 +120,27 @@ eth0_mdio: eth0-mdio-pins {
samsung,pin-pud = <FSD_PIN_PULL_NONE>;
samsung,pin-drv = <FSD_PIN_DRV_LV4>;
};
+
+ pcie1_clkreq: pcie1-clkreq-pins {
+ samsung,pins = "gpf6-0";
+ samsung,pin-function = <FSD_PIN_FUNC_2>;
+ samsung,pin-pud = <FSD_PIN_PULL_UP>;
+ samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+ };
+
+ pcie1_wake: pcie1-wake-pins {
+ samsung,pins = "gpf6-1";
+ samsung,pin-function = <FSD_PIN_FUNC_2>;
+ samsung,pin-pud = <FSD_PIN_PULL_UP>;
+ samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+ };
+
+ pcie1_preset: pcie1-preset-pins {
+ samsung,pins = "gpf6-2";
+ samsung,pin-function = <FSD_PIN_FUNC_2>;
+ samsung,pin-pud = <FSD_PIN_PULL_UP>;
+ samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+ };
};
&pinctrl_peric {
@@ -493,6 +514,50 @@ eth1_mdio: eth1-mdio-pins {
samsung,pin-pud = <FSD_PIN_PULL_UP>;
samsung,pin-drv = <FSD_PIN_DRV_LV4>;
};
+
+ pcie0_clkreq: pcie0-clkreq-pins {
+ samsung,pins = "gpc8-0";
+ samsung,pin-function = <FSD_PIN_FUNC_2>;
+ samsung,pin-pud = <FSD_PIN_PULL_UP>;
+ samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+ };
+
+ pcie0_wake0: pcie0-wake0-pins {
+ samsung,pins = "gpc8-1";
+ samsung,pin-function = <FSD_PIN_FUNC_2>;
+ samsung,pin-pud = <FSD_PIN_PULL_UP>;
+ samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+ };
+
+ pcie0_preset0: pcie0-preset0-pins {
+ samsung,pins = "gpc8-2";
+ samsung,pin-function = <FSD_PIN_FUNC_2>;
+ samsung,pin-pud = <FSD_PIN_PULL_UP>;
+ samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+ };
+
+ pcie0_wake1: pcie0-wake1-pins {
+ samsung,pins = "gpc8-3";
+ samsung,pin-function = <FSD_PIN_FUNC_2>;
+ samsung,pin-pud = <FSD_PIN_PULL_UP>;
+ samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+ };
+
+ pcie0_slot0: pcie0-gpio22-pins {
+ samsung,pins = "gpg2-6";
+ samsung,pin-function = <FSD_PIN_FUNC_OUTPUT>;
+ samsung,pin-pud = <FSD_PIN_PULL_UP>;
+ samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+ samsung,pin-val = <1>;
+ };
+
+ pcie0_slot1: pcie0-gpio23-pins {
+ samsung,pins = "gpg2-7";
+ samsung,pin-function = <FSD_PIN_FUNC_OUTPUT>;
+ samsung,pin-pud = <FSD_PIN_PULL_UP>;
+ samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+ samsung,pin-val = <1>;
+ };
};
&pinctrl_pmu {
diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
index a5ebb3f9b18f..8ed8d2131855 100644
--- a/arch/arm64/boot/dts/tesla/fsd.dtsi
+++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
@@ -1009,6 +1009,16 @@ ethernet1: ethernet@14300000 {
status = "disabled";
};
+ pciephy0: pcie-phy@15080000 {
+ compatible = "tesla,fsd-pcie-phy0";
+ reg = <0x0 0x15080000 0x0 0x2000>,
+ <0x0 0x150a0000 0x0 0x1000>;
+ #phy-cells = <0>;
+ samsung,pmu-syscon = <&pmu_system_controller>;
+ samsung,fsys-sysreg = <&sysreg_fsys0>;
+ status = "disabled";
+ };
+
ufs: ufs@15120000 {
compatible = "tesla,fsd-ufs";
reg = <0x0 0x15120000 0x0 0x200>, /* 0: HCI standard */
@@ -1057,6 +1067,143 @@ ethernet0: ethernet@15300000 {
iommus = <&smmu_fsys0 0x0 0x1>;
status = "disabled";
};
+
+ pcierc2: pcie@15400000 {
+ compatible = "tesla,fsd-pcie";
+ reg = <0x0 0x15400000 0x0 0x2000>,
+ <0x0 0x15090000 0x0 0x1000>,
+ <0x0 0x15800000 0x0 0x1000>;
+ reg-names = "dbi", "elbi", "config";
+ ranges = <0x82000000 0 0x15801000 0 0x15801000 0 0xffefff>;
+ clocks = <&clock_fsys0 PCIE_SUBCTRL_INST0_AUX_CLK_SOC>,
+ <&clock_fsys0 PCIE_SUBCTRL_INST0_DBI_ACLK_SOC>,
+ <&clock_fsys0 PCIE_SUBCTRL_INST0_MSTR_ACLK_SOC>,
+ <&clock_fsys0 PCIE_SUBCTRL_INST0_SLV_ACLK_SOC>;
+ clock-names = "aux", "dbi", "mstr", "slv";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ dma-coherent;
+ device_type = "pci";
+ interrupts = <GIC_SPI 93 IRQ_TYPE_EDGE_RISING>;
+ num-lanes = <4>;
+ phys = <&pciephy0>;
+ iommu-map = <0x0 &smmu_fsys0 0x4 0x10000>;
+ iommu-map-mask = <0x0>;
+ samsung,syscon-pcie = <&sysreg_fsys0 0x434>;
+ status = "disabled";
+ };
+
+ pcieep2: pcie-ep@15400000 {
+ compatible = "tesla,fsd-pcie-ep";
+ reg = <0x0 0x15090000 0x0 0x1000>,
+ <0x0 0x15400000 0x0 0x2000>,
+ <0x0 0x15402000 0x0 0x80>,
+ <0x0 0x15800000 0x0 0xff0000>;
+ reg-names = "elbi", "dbi", "dbi2", "addr_space";
+ clocks = <&clock_fsys0 PCIE_SUBCTRL_INST0_AUX_CLK_SOC>,
+ <&clock_fsys0 PCIE_SUBCTRL_INST0_DBI_ACLK_SOC>,
+ <&clock_fsys0 PCIE_SUBCTRL_INST0_MSTR_ACLK_SOC>,
+ <&clock_fsys0 PCIE_SUBCTRL_INST0_SLV_ACLK_SOC>;
+ clock-names = "aux", "dbi", "mstr", "slv";
+ num-lanes = <4>;
+ phys = <&pciephy0>;
+ samsung,syscon-pcie = <&sysreg_fsys0 0x434>;
+ status = "disabled";
+ };
+
+ pciephy1: pcie-phy@16880000 {
+ compatible = "tesla,fsd-pcie-phy1";
+ reg = <0x0 0x16880000 0x0 0x2000>,
+ <0x0 0x16860000 0x0 0x1000>;
+ #phy-cells = <0>;
+ samsung,pmu-syscon = <&pmu_system_controller>;
+ samsung,fsys-sysreg = <&sysreg_fsys1>;
+ status = "disabled";
+ };
+
+ pcierc0: pcie@16a00000 {
+ compatible = "tesla,fsd-pcie";
+ reg = <0x0 0x16a00000 0x0 0x2000>,
+ <0x0 0x168b0000 0x0 0x1000>,
+ <0x0 0x17000000 0x0 0x1000>;
+ reg-names = "dbi", "elbi", "config";
+ ranges = <0x82000000 0 0x17001000 0 0x17001000 0 0xffefff>;
+ clocks = <&clock_fsys1 PCIE_LINK0_IPCLKPORT_AUX_ACLK>,
+ <&clock_fsys1 PCIE_LINK0_IPCLKPORT_DBI_ACLK>,
+ <&clock_fsys1 PCIE_LINK0_IPCLKPORT_MSTR_ACLK>,
+ <&clock_fsys1 PCIE_LINK0_IPCLKPORT_SLV_ACLK>;
+ clock-names = "aux", "dbi", "mstr", "slv";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ dma-coherent;
+ device_type = "pci";
+ interrupts = <GIC_SPI 115 IRQ_TYPE_EDGE_RISING>;
+ num-lanes = <4>;
+ phys = <&pciephy1>;
+ iommu-map = <0x0 &smmu_imem 0x0 0x10000>;
+ iommu-map-mask = <0x0>;
+ samsung,syscon-pcie = <&sysreg_fsys1 0x50c>;
+ status = "disabled";
+ };
+
+ pcieep0: pcie-ep@16a00000 {
+ compatible = "tesla,fsd-pcie-ep";
+ reg = <0x0 0x168b0000 0x0 0x1000>,
+ <0x0 0x16a00000 0x0 0x2000>,
+ <0x0 0x16a02000 0x0 0x80>,
+ <0x0 0x17000000 0x0 0xff0000>;
+ reg-names = "elbi", "dbi", "dbi2", "addr_space";
+ clocks = <&clock_fsys1 PCIE_LINK0_IPCLKPORT_AUX_ACLK>,
+ <&clock_fsys1 PCIE_LINK0_IPCLKPORT_DBI_ACLK>,
+ <&clock_fsys1 PCIE_LINK0_IPCLKPORT_MSTR_ACLK>,
+ <&clock_fsys1 PCIE_LINK0_IPCLKPORT_SLV_ACLK>;
+ clock-names = "aux", "dbi", "mstr", "slv";
+ num-lanes = <4>;
+ phys = <&pciephy1>;
+ samsung,syscon-pcie = <&sysreg_fsys1 0x50c>;
+ status = "disabled";
+ };
+
+ pcierc1: pcie@16b00000 {
+ compatible = "tesla,fsd-pcie";
+ reg = <0x0 0x16b00000 0x0 0x2000>,
+ <0x0 0x168c0000 0x0 0x1000>,
+ <0x0 0x18000000 0x0 0x1000>;
+ reg-names = "dbi", "elbi", "config";
+ ranges = <0x82000000 0 0x18001000 0 0x18001000 0 0xffefff>;
+ clocks = <&clock_fsys1 PCIE_LINK1_IPCLKPORT_AUX_ACLK>,
+ <&clock_fsys1 PCIE_LINK1_IPCLKPORT_DBI_ACLK>,
+ <&clock_fsys1 PCIE_LINK1_IPCLKPORT_MSTR_ACLK>,
+ <&clock_fsys1 PCIE_LINK1_IPCLKPORT_SLV_ACLK>;
+ clock-names = "aux", "dbi", "mstr", "slv";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ dma-coherent;
+ device_type = "pci";
+ interrupts = <GIC_SPI 117 IRQ_TYPE_EDGE_RISING>;
+ num-lanes = <4>;
+ phys = <&pciephy1>;
+ samsung,syscon-pcie = <&sysreg_fsys1 0x510>;
+ status = "disabled";
+ };
+
+ pcieep1: pcie-ep@16b00000 {
+ compatible = "tesla,fsd-pcie-ep";
+ reg = <0x0 0x168c0000 0x0 0x1000>,
+ <0x0 0x16b00000 0x0 0x2000>,
+ <0x0 0x16b02000 0x0 0x80>,
+ <0x0 0x18000000 0x0 0xff0000>;
+ reg-names = "elbi", "dbi", "dbi2", "addr_space";
+ clocks = <&clock_fsys1 PCIE_LINK1_IPCLKPORT_AUX_ACLK>,
+ <&clock_fsys1 PCIE_LINK1_IPCLKPORT_DBI_ACLK>,
+ <&clock_fsys1 PCIE_LINK1_IPCLKPORT_MSTR_ACLK>,
+ <&clock_fsys1 PCIE_LINK1_IPCLKPORT_SLV_ACLK>;
+ clock-names = "aux", "dbi", "mstr", "slv";
+ num-lanes = <4>;
+ phys = <&pciephy1>;
+ samsung,syscon-pcie = <&sysreg_fsys1 0x510>;
+ status = "disabled";
+ };
};
};
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 12/12] arm64: dts: fsd: Add PCIe support for Tesla FSD SoC
2025-08-11 15:46 ` [PATCH v3 12/12] arm64: dts: fsd: Add PCIe " Shradha Todi
@ 2025-08-12 6:43 ` Krzysztof Kozlowski
2025-08-18 8:54 ` Shradha Todi
2025-08-30 3:58 ` Manivannan Sadhasivam
1 sibling, 1 reply; 34+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-12 6:43 UTC (permalink / raw)
To: Shradha Todi, linux-pci, devicetree, linux-arm-kernel,
linux-samsung-soc, linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey
On 11/08/2025 17:46, Shradha Todi wrote:
> Add the support for PCIe controller driver and phy driver for Tesla FSD.
> It includes support for both RC and EP.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
> arch/arm64/boot/dts/tesla/fsd-evb.dts | 34 +++++
> arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 65 +++++++++
> arch/arm64/boot/dts/tesla/fsd.dtsi | 147 +++++++++++++++++++++
> 3 files changed, 246 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> index 9ff22e1c8723..1b63c5d72d19 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> @@ -130,3 +130,37 @@ &serial_0 {
> &ufs {
> status = "okay";
> };
> +
> +&pcierc2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> + <&pcie0_slot1>;
> +};
> +
> +&pcieep2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> + <&pcie0_slot1>;
> +};
> +
> +&pcierc0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> + <&pcie0_slot0>;
> +};
> +
> +&pcieep0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> + <&pcie0_slot0>;
> +};
> +
> +&pcierc1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
> +};
> +
> +&pcieep1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
All these are pointless, because the node is disabled. The board level
should be complete, so also supplies and enabling the device.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 34+ messages in thread* RE: [PATCH v3 12/12] arm64: dts: fsd: Add PCIe support for Tesla FSD SoC
2025-08-12 6:43 ` Krzysztof Kozlowski
@ 2025-08-18 8:54 ` Shradha Todi
0 siblings, 0 replies; 34+ messages in thread
From: Shradha Todi @ 2025-08-18 8:54 UTC (permalink / raw)
To: 'Krzysztof Kozlowski', linux-pci, devicetree,
linux-arm-kernel, linux-samsung-soc, linux-kernel, linux-phy
Cc: mani, lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd, m.szyprowski,
jh80.chung, pankaj.dubey
> > +&pcieep2 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> > + <&pcie0_slot1>;
> > +};
> > +
> > +&pcierc0 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> > + <&pcie0_slot0>;
> > +};
> > +
> > +&pcieep0 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> > + <&pcie0_slot0>;
> > +};
> > +
> > +&pcierc1 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
> > +};
> > +
> > +&pcieep1 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
>
>
> All these are pointless, because the node is disabled. The board level
> should be complete, so also supplies and enabling the device.
>
I will enable required nodes. Had enabled while testing but missed to
add in patch. Though all nodes will not be enabled as it is a dual-mode
controller and cannot run as both RC and EP at the same time.
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 12/12] arm64: dts: fsd: Add PCIe support for Tesla FSD SoC
2025-08-11 15:46 ` [PATCH v3 12/12] arm64: dts: fsd: Add PCIe " Shradha Todi
2025-08-12 6:43 ` Krzysztof Kozlowski
@ 2025-08-30 3:58 ` Manivannan Sadhasivam
1 sibling, 0 replies; 34+ messages in thread
From: Manivannan Sadhasivam @ 2025-08-30 3:58 UTC (permalink / raw)
To: Shradha Todi
Cc: linux-pci, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-phy, lpieralisi, kwilczynski, robh, bhelgaas,
jingoohan1, krzk+dt, conor+dt, alim.akhtar, vkoul, kishon, arnd,
m.szyprowski, jh80.chung, pankaj.dubey
On Mon, Aug 11, 2025 at 09:16:38PM GMT, Shradha Todi wrote:
> Add the support for PCIe controller driver and phy driver for Tesla FSD.
> It includes support for both RC and EP.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
> arch/arm64/boot/dts/tesla/fsd-evb.dts | 34 +++++
> arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 65 +++++++++
> arch/arm64/boot/dts/tesla/fsd.dtsi | 147 +++++++++++++++++++++
> 3 files changed, 246 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> index 9ff22e1c8723..1b63c5d72d19 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> @@ -130,3 +130,37 @@ &serial_0 {
> &ufs {
> status = "okay";
> };
> +
> +&pcierc2 {
It'd be good to use underscore to differentiate RC and EP modes:
pcie_rc1
pcie_ep1
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> + <&pcie0_slot1>;
Could you please explain what these 'preset' and 'slot' pins are?
> +};
> +
> +&pcieep2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> + <&pcie0_slot1>;
> +};
> +
> +&pcierc0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> + <&pcie0_slot0>;
> +};
> +
> +&pcieep0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> + <&pcie0_slot0>;
> +};
> +
> +&pcierc1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
> +};
> +
> +&pcieep1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
> +};
[...]
> + pcieep2: pcie-ep@15400000 {
> + compatible = "tesla,fsd-pcie-ep";
> + reg = <0x0 0x15090000 0x0 0x1000>,
> + <0x0 0x15400000 0x0 0x2000>,
> + <0x0 0x15402000 0x0 0x80>,
> + <0x0 0x15800000 0x0 0xff0000>;
> + reg-names = "elbi", "dbi", "dbi2", "addr_space";
> + clocks = <&clock_fsys0 PCIE_SUBCTRL_INST0_AUX_CLK_SOC>,
> + <&clock_fsys0 PCIE_SUBCTRL_INST0_DBI_ACLK_SOC>,
> + <&clock_fsys0 PCIE_SUBCTRL_INST0_MSTR_ACLK_SOC>,
> + <&clock_fsys0 PCIE_SUBCTRL_INST0_SLV_ACLK_SOC>;
> + clock-names = "aux", "dbi", "mstr", "slv";
> + num-lanes = <4>;
> + phys = <&pciephy0>;
> + samsung,syscon-pcie = <&sysreg_fsys0 0x434>;
> + status = "disabled";
So only host mode DMA is cache coherent and not endpoint? Weird.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 34+ messages in thread