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 | 331 +++++++++++++++++++++++-
1 file changed, 323 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index dff23cf648f5..5b525156b9f5 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -20,6 +20,8 @@
#include <linux/regulator/consumer.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
#include "pcie-designware.h"
@@ -49,17 +51,46 @@
#define EXYNOS_PCIE_ELBI_SLV_ARMISC 0x120
#define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE BIT(21)
+#define FSD_IRQ2_STS 0x008
+#define FSD_IRQ_MSI_ENABLE BIT(17)
+#define FSD_IRQ2_EN 0x018
+#define FSD_PCIE_APP_LTSSM_ENABLE 0x054
+#define FSD_PCIE_LTSSM_ENABLE 0x1
+#define FSD_PCIE_DEVICE_TYPE 0x080
+#define FSD_DEVICE_TYPE_RC 0x4
+#define FSD_DEVICE_TYPE_EP 0x0
+#define FSD_PCIE_CXPL_DEBUG_00_31 0x2C8
+
+/* to store different SoC variants of Samsung */
+enum samsung_pcie_variants {
+ FSD,
+ EXYNOS_5433,
+};
+
+/* Values to be written to SYSREG to view DBI space as CDM/DBI2/IATU/DMA */
+enum fsd_pcie_addr_type {
+ ADDR_TYPE_DBI = 0x0,
+ ADDR_TYPE_DBI2 = 0x12,
+ ADDR_TYPE_ATU = 0x36,
+ ADDR_TYPE_DMA = 0x3f,
+};
+
struct samsung_pcie_pdata {
struct pci_ops *pci_ops;
const struct dw_pcie_ops *dwc_ops;
const struct dw_pcie_host_ops *host_ops;
+ const struct dw_pcie_ep_ops *ep_ops;
const struct samsung_res_ops *res_ops;
+ unsigned int soc_variant;
+ enum dw_pcie_device_mode device_mode;
};
struct exynos_pcie {
struct dw_pcie pci;
void __iomem *elbi_base;
const struct samsung_pcie_pdata *pdata;
+ struct regmap *sysreg;
+ unsigned int sysreg_offset;
struct clk_bulk_data *clks;
struct phy *phy;
struct regulator_bulk_data *supplies;
@@ -69,6 +100,7 @@ struct exynos_pcie {
struct samsung_res_ops {
int (*init_regulator)(struct exynos_pcie *ep);
irqreturn_t (*pcie_irq_handler)(int irq, void *arg);
+ void (*set_device_mode)(struct exynos_pcie *ep);
};
static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
@@ -326,11 +358,203 @@ 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 MSIX\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;
@@ -353,6 +577,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))
@@ -373,13 +617,43 @@ static int exynos_pcie_probe(struct platform_device *pdev)
return ret;
platform_set_drvdata(pdev, ep);
- ret = samsung_irq_init(ep, pdev);
- if (ret)
- goto fail_regulator;
- ep->pci.pp.ops = pdata->host_ops;
- ret = dw_pcie_host_init(&ep->pci.pp);
- if (ret < 0)
+
+ if (pdata->res_ops->set_device_mode)
+ pdata->res_ops->set_device_mode(ep);
+
+ switch (ep->pdata->device_mode) {
+ case DW_PCIE_RC_TYPE:
+ ret = samsung_irq_init(ep, pdev);
+ if (ret)
+ goto fail_regulator;
+
+ ep->pci.pp.ops = pdata->host_ops;
+
+ ret = dw_pcie_host_init(&ep->pci.pp);
+ if (ret < 0)
+ goto fail_phy_init;
+
+ break;
+ case DW_PCIE_EP_TYPE:
+ phy_init(ep->phy);
+
+ ep->pci.ep.ops = pdata->ep_ops;
+
+ ret = dw_pcie_ep_init(&ep->pci.ep);
+ if (ret < 0)
+ goto fail_phy_init;
+
+ ret = dw_pcie_ep_init_registers(&ep->pci.ep);
+ if (ret)
+ goto fail_phy_init;
+
+ pci_epc_init_notify(ep->pci.ep.epc);
+
+ break;
+ default:
+ dev_err(dev, "invalid device type\n");
goto fail_phy_init;
+ }
return 0;
@@ -395,8 +669,11 @@ static void exynos_pcie_remove(struct platform_device *pdev)
{
struct exynos_pcie *ep = platform_get_drvdata(pdev);
+ if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
+ return;
dw_pcie_host_deinit(&ep->pci.pp);
- exynos_pcie_assert_core_reset(ep);
+ if (ep->pdata->soc_variant == EXYNOS_5433)
+ exynos_pcie_assert_core_reset(ep);
phy_power_off(ep->phy);
phy_exit(ep->phy);
samsung_regulator_disable(ep);
@@ -405,8 +682,16 @@ static void exynos_pcie_remove(struct platform_device *pdev)
static int exynos_pcie_suspend_noirq(struct device *dev)
{
struct exynos_pcie *ep = dev_get_drvdata(dev);
+ struct dw_pcie *pci = &ep->pci;
- exynos_pcie_assert_core_reset(ep);
+ if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
+ return 0;
+
+ if (ep->pdata->dwc_ops->stop_link)
+ ep->pdata->dwc_ops->stop_link(pci);
+
+ if (ep->pdata->soc_variant == EXYNOS_5433)
+ exynos_pcie_assert_core_reset(ep);
phy_power_off(ep->phy);
phy_exit(ep->phy);
samsung_regulator_disable(ep);
@@ -421,6 +706,9 @@ static int exynos_pcie_resume_noirq(struct device *dev)
struct dw_pcie_rp *pp = &pci->pp;
int ret;
+ if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
+ return 0;
+
ret = samsung_regulator_enable(ep);
if (ret)
return ret;
@@ -437,11 +725,30 @@ static const struct dev_pm_ops exynos_pcie_pm_ops = {
exynos_pcie_resume_noirq)
};
+
+static const struct samsung_pcie_pdata fsd_hw3_pcie_rc_pdata = {
+ .dwc_ops = &fsd_dw_pcie_ops,
+ .host_ops = &fsd_pcie_host_ops,
+ .res_ops = &fsd_res_ops_data,
+ .soc_variant = FSD,
+ .device_mode = DW_PCIE_RC_TYPE,
+};
+
+static const struct samsung_pcie_pdata fsd_hw3_pcie_ep_pdata = {
+ .dwc_ops = &fsd_dw_pcie_ops,
+ .ep_ops = &fsd_ep_ops,
+ .res_ops = &fsd_res_ops_data,
+ .soc_variant = FSD,
+ .device_mode = DW_PCIE_EP_TYPE,
+};
+
static const struct samsung_pcie_pdata exynos_5433_pcie_rc_pdata = {
.dwc_ops = &exynos_dw_pcie_ops,
.pci_ops = &exynos_pci_ops,
.host_ops = &exynos_pcie_host_ops,
.res_ops = &exynos_res_ops_data,
+ .soc_variant = EXYNOS_5433,
+ .device_mode = DW_PCIE_RC_TYPE,
};
static const struct of_device_id exynos_pcie_of_match[] = {
@@ -449,6 +756,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.0Hi Shradha,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Shradha-Todi/PCI-exynos-Remove-unused-MACROs-in-exynos-PCI-file/20250626-104154
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20250625165229.3458-10-shradha.t%40samsung.com
patch subject: [PATCH v2 09/10] PCI: exynos: Add support for Tesla FSD SoC
config: um-randconfig-r071-20250630 (https://download.01.org/0day-ci/archive/20250630/202506301329.VWoiH0yn-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202506301329.VWoiH0yn-lkp@intel.com/
smatch warnings:
drivers/pci/controller/dwc/pci-exynos.c:621 exynos_pcie_probe() error: we previously assumed 'pdata->res_ops' could be null (see line 609)
drivers/pci/controller/dwc/pci-exynos.c:655 exynos_pcie_probe() warn: missing error code 'ret'
vim +621 drivers/pci/controller/dwc/pci-exynos.c
b9388ee21b4e79 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 595 dev_err(dev, "couldn't get the register offset for syscon!\n");
b9388ee21b4e79 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 596 return ret;
b9388ee21b4e79 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 597 }
b9388ee21b4e79 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 598 }
b9388ee21b4e79 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 599
778f7c194b1dac drivers/pci/controller/dwc/pci-exynos.c Jaehoon Chung 2020-11-13 600 /* External Local Bus interface (ELBI) registers */
778f7c194b1dac drivers/pci/controller/dwc/pci-exynos.c Jaehoon Chung 2020-11-13 601 ep->elbi_base = devm_platform_ioremap_resource_byname(pdev, "elbi");
778f7c194b1dac drivers/pci/controller/dwc/pci-exynos.c Jaehoon Chung 2020-11-13 602 if (IS_ERR(ep->elbi_base))
778f7c194b1dac drivers/pci/controller/dwc/pci-exynos.c Jaehoon Chung 2020-11-13 603 return PTR_ERR(ep->elbi_base);
778f7c194b1dac drivers/pci/controller/dwc/pci-exynos.c Jaehoon Chung 2020-11-13 604
10106d5c1f9cee drivers/pci/controller/dwc/pci-exynos.c Cristian Ciocaltea 2024-12-17 605 ret = devm_clk_bulk_get_all_enabled(dev, &ep->clks);
6b11143f9344dd dripdata->res_opsvers/pci/controller/dwc/pci-exynos.c Shradha Todi 2024-02-20 606 if (ret < 0)
6b11143f9344dd drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2024-02-20 607 return ret;
4b1ced841b2e31 drivers/pci/host/pci-exynos.c Jingoo Han 2013-07-31 608
ed1b6ec2c47ce8 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 @609 if (pdata->res_ops && pdata->res_ops->init_regulator) {
^^^^^^^^^^^^^^
This code assumes pdata->res_ops can be NULL
ed1b6ec2c47ce8 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 610 ret = ep->pdata->res_ops->init_regulator(ep);
4b1ced841b2e31 drivers/pci/host/pci-exynos.c Jingoo Han 2013-07-31 611 if (ret)
4b1ced841b2e31 drivers/pci/host/pci-exynos.c Jingoo Han 2013-07-31 612 return ret;
ed1b6ec2c47ce8 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 613 }
4b1ced841b2e31 drivers/pci/host/pci-exynos.c Jingoo Han 2013-07-31 614
ed1b6ec2c47ce8 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 615 ret = samsung_regulator_enable(ep);
3278478084747c drivers/pci/host/pci-exynos.c Niyas Ahmed S T 2017-02-01 616 if (ret)
3278478084747c drivers/pci/host/pci-exynos.c Niyas Ahmed S T 2017-02-01 617 return ret;
4b1ced841b2e31 drivers/pci/host/pci-exynos.c Jingoo Han 2013-07-31 618
b2e6d3055d5545 drivers/pci/dwc/pci-exynos.c Bjorn Helgaas 2017-02-21 619 platform_set_drvdata(pdev, ep);
b9388ee21b4e79 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 620
b9388ee21b4e79 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 @621 if (pdata->res_ops->set_device_mode)
^^^^^^^^^^^^^^
But this dereferences it without checking. Most likely the
NULL check should be removed?
b9388ee21b4e79 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 622 pdata->res_ops->set_device_mode(ep);
b9388ee21b4e79 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 623
b9388ee21b4e79 drivers/pci/controller/dwc/pci-exynos.c Shradha Todi 2025-06-25 624 switch (ep->pdata->device_mode) {
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Wed, Jun 25, 2025 at 10:22:28PM +0530, Shradha Todi wrote:
> Add host and endpoint controller driver support for FSD SoC.
> +++ b/drivers/pci/controller/dwc/pci-exynos.c
> @@ -20,6 +20,8 @@
> #include <linux/regulator/consumer.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
The trend is to sort these alphabetically. The last couple additions
didn't observe this, but maybe these new ones could go a little
farther up and make it more sorted rather than less?
> +#define FSD_PCIE_CXPL_DEBUG_00_31 0x2C8
Existing #defines use lower-case hex; please follow suit.
> +/* to store different SoC variants of Samsung */
> +enum samsung_pcie_variants {
> + FSD,
> + 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 dw_pcie_ep_ops *ep_ops;
> const struct samsung_res_ops *res_ops;
> + unsigned int soc_variant;
> + enum dw_pcie_device_mode device_mode;
> };
> +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;
Remove blank lines to match style of fsd_pcie_write_dbi2().
> +}
> +
> +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);
Ditto.
> +}
> +
> +static void fsd_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size, u32 val)
> +{
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> + fsd_atu_setting(pci, base);
> + dw_pcie_write(pci->dbi_base + reg, size, val);
> + regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
> +}
> +static int fsd_pcie_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 MSIX\n");
s/MSIX/MSI-X/ to match spec usage.
> @@ -373,13 +617,43 @@ static int exynos_pcie_probe(struct platform_device *pdev)
> return ret;
>
> platform_set_drvdata(pdev, ep);
> - ret = samsung_irq_init(ep, pdev);
> - if (ret)
> - goto fail_regulator;
> - ep->pci.pp.ops = pdata->host_ops;
> - ret = dw_pcie_host_init(&ep->pci.pp);
> - if (ret < 0)
> +
> + if (pdata->res_ops->set_device_mode)
> + pdata->res_ops->set_device_mode(ep);
> +
> + switch (ep->pdata->device_mode) {
> + case DW_PCIE_RC_TYPE:
> + ret = samsung_irq_init(ep, pdev);
> + if (ret)
> + goto fail_regulator;
> +
> + ep->pci.pp.ops = pdata->host_ops;
> +
> + ret = dw_pcie_host_init(&ep->pci.pp);
> + if (ret < 0)
> + goto fail_phy_init;
> +
> + break;
> + case DW_PCIE_EP_TYPE:
> + phy_init(ep->phy);
> +
> + ep->pci.ep.ops = pdata->ep_ops;
> +
> + ret = dw_pcie_ep_init(&ep->pci.ep);
> + if (ret < 0)
> + goto fail_phy_init;
> +
> + ret = dw_pcie_ep_init_registers(&ep->pci.ep);
> + if (ret)
> + goto fail_phy_init;
> +
> + pci_epc_init_notify(ep->pci.ep.epc);
> +
> + break;
> + default:
> + dev_err(dev, "invalid device type\n");
> goto fail_phy_init;
> + }
This would be a little nicer if you added soc_variant and device_mode
and the code that sets and tests them for exynos_5433 first in a
separate patch. Then it would be more obvious that the new FSD parts
don't affect exynos_5433 since this patch would only be *adding*
FSD-specific things.
> 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[] = {
> @@ -449,6 +756,14 @@ static const struct of_device_id exynos_pcie_of_match[] = {
> .compatible = "samsung,exynos5433-pcie",
> .data = (void *) &exynos_5433_pcie_rc_pdata,
> },
> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 28 June 2025 01:01
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
linux-
> samsung-soc@vger.kernel.org; linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org; linux-
> fsd@tesla.com; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org; kw@linux.com;
> robh@kernel.org; bhelgaas@google.com; jingoohan1@gmail.com; krzk+dt@kernel.org;
> conor+dt@kernel.org; alim.akhtar@samsung.com; vkoul@kernel.org; kishon@kernel.org;
> arnd@arndb.de; m.szyprowski@samsung.com; jh80.chung@samsung.com; pankaj.dubey@samsung.com
> Subject: Re: [PATCH v2 09/10] PCI: exynos: Add support for Tesla FSD SoC
>
> On Wed, Jun 25, 2025 at 10:22:28PM +0530, Shradha Todi wrote:
> > Add host and endpoint controller driver support for FSD SoC.
>
> > +++ b/drivers/pci/controller/dwc/pci-exynos.c
> > @@ -20,6 +20,8 @@
> > #include <linux/regulator/consumer.h>
> > #include <linux/mod_devicetable.h>
> > #include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <linux/mfd/syscon.h>
>
> The trend is to sort these alphabetically. The last couple additions
> didn't observe this, but maybe these new ones could go a little
> farther up and make it more sorted rather than less?
>
> > +#define FSD_PCIE_CXPL_DEBUG_00_31 0x2C8
>
> Existing #defines use lower-case hex; please follow suit.
>
> > +/* to store different SoC variants of Samsung */
> > +enum samsung_pcie_variants {
> > + FSD,
> > + 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 dw_pcie_ep_ops *ep_ops;
> > const struct samsung_res_ops *res_ops;
> > + unsigned int soc_variant;
> > + enum dw_pcie_device_mode device_mode;
> > };
>
> > +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;
>
> Remove blank lines to match style of fsd_pcie_write_dbi2().
>
> > +}
> > +
> > +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);
>
> Ditto.
>
> > +}
> > +
> > +static void fsd_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base,
> > + u32 reg, size_t size, u32 val)
> > +{
> > + struct exynos_pcie *ep = to_exynos_pcie(pci);
> > +
> > + fsd_atu_setting(pci, base);
> > + dw_pcie_write(pci->dbi_base + reg, size, val);
> > + regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
> > +}
>
> > +static int fsd_pcie_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 MSIX\n");
>
> s/MSIX/MSI-X/ to match spec usage.
>
Thanks for the review! Will take care of all mentioned changes in next version
> > @@ -373,13 +617,43 @@ static int exynos_pcie_probe(struct platform_device *pdev)
> > return ret;
> >
> > platform_set_drvdata(pdev, ep);
> > - ret = samsung_irq_init(ep, pdev);
> > - if (ret)
> > - goto fail_regulator;
> > - ep->pci.pp.ops = pdata->host_ops;
> > - ret = dw_pcie_host_init(&ep->pci.pp);
> > - if (ret < 0)
> > +
> > + if (pdata->res_ops->set_device_mode)
> > + pdata->res_ops->set_device_mode(ep);
> > +
> > + switch (ep->pdata->device_mode) {
> > + case DW_PCIE_RC_TYPE:
> > + ret = samsung_irq_init(ep, pdev);
> > + if (ret)
> > + goto fail_regulator;
> > +
> > + ep->pci.pp.ops = pdata->host_ops;
> > +
> > + ret = dw_pcie_host_init(&ep->pci.pp);
> > + if (ret < 0)
> > + goto fail_phy_init;
> > +
> > + break;
> > + case DW_PCIE_EP_TYPE:
> > + phy_init(ep->phy);
> > +
> > + ep->pci.ep.ops = pdata->ep_ops;
> > +
> > + ret = dw_pcie_ep_init(&ep->pci.ep);
> > + if (ret < 0)
> > + goto fail_phy_init;
> > +
> > + ret = dw_pcie_ep_init_registers(&ep->pci.ep);
> > + if (ret)
> > + goto fail_phy_init;
> > +
> > + pci_epc_init_notify(ep->pci.ep.epc);
> > +
> > + break;
> > + default:
> > + dev_err(dev, "invalid device type\n");
> > goto fail_phy_init;
> > + }
>
> This would be a little nicer if you added soc_variant and device_mode
> and the code that sets and tests them for exynos_5433 first in a
> separate patch. Then it would be more obvious that the new FSD parts
> don't affect exynos_5433 since this patch would only be *adding*
> FSD-specific things.
>
Sure, I have no issues in splitting the patches further. Though unfortunately,
I or anyone I know does not possess a board which has Exynos 5433 chipset.
Therefore, I'm unable to verify these changes for Exynos chipset. I took care
to not disturb the exynos flow functionally but would be great if someone
could test this and confirm that it works well on Exynos 5433 after the changes.
> > 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[] = {
> > @@ -449,6 +756,14 @@ static const struct of_device_id exynos_pcie_of_match[] = {
> > .compatible = "samsung,exynos5433-pcie",
> > .data = (void *) &exynos_5433_pcie_rc_pdata,
> > },
On Tue, Jul 01, 2025 at 04:48:13PM +0530, Shradha Todi wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>
> > Sent: 28 June 2025 01:01
> > To: Shradha Todi <shradha.t@samsung.com>
> > Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-
> > samsung-soc@vger.kernel.org; linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org; linux-
> > fsd@tesla.com; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org; kw@linux.com;
> > robh@kernel.org; bhelgaas@google.com; jingoohan1@gmail.com; krzk+dt@kernel.org;
> > conor+dt@kernel.org; alim.akhtar@samsung.com; vkoul@kernel.org; kishon@kernel.org;
> > arnd@arndb.de; m.szyprowski@samsung.com; jh80.chung@samsung.com; pankaj.dubey@samsung.com
> > Subject: Re: [PATCH v2 09/10] PCI: exynos: Add support for Tesla FSD SoC
Would be good if your mailer could support the usual quote mechanism,
e.g. the "On Wed, Jun 25, 2025 at 10:22:28PM +0530, Shradha Todi
wrote:" line below. No need for all the header duplication above.
> > On Wed, Jun 25, 2025 at 10:22:28PM +0530, Shradha Todi wrote:
> > > Add host and endpoint controller driver support for FSD SoC.
> > This would be a little nicer if you added soc_variant and device_mode
> > and the code that sets and tests them for exynos_5433 first in a
> > separate patch. Then it would be more obvious that the new FSD parts
> > don't affect exynos_5433 since this patch would only be *adding*
> > FSD-specific things.
>
> Sure, I have no issues in splitting the patches further. Though
> unfortunately, I or anyone I know does not possess a board which has
> Exynos 5433 chipset. Therefore, I'm unable to verify these changes
> for Exynos chipset. I took care to not disturb the exynos flow
> functionally but would be great if someone could test this and
> confirm that it works well on Exynos 5433 after the changes.
Yeah, that's a common situation, and there's no problem with adding
this functionality. But in the unlikely event there's a mistake that
relates to Exynos 5433, it will be easier for someone with that board
to analyze the problem if the changes that affect exynos_5433 are
split out into a patch that doesn't include any FSD-related changes.
> > > 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[] = {
> > > @@ -449,6 +756,14 @@ static const struct of_device_id exynos_pcie_of_match[] = {
> > > .compatible = "samsung,exynos5433-pcie",
> > > .data = (void *) &exynos_5433_pcie_rc_pdata,
> > > },
>
© 2016 - 2025 Red Hat, Inc.