[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4e3a959c.9ca.19ac01e156f.Coremail.zhangsenchuan@eswincomputing.com>
Date: Wed, 26 Nov 2025 20:23:08 +0800 (GMT+08:00)
From: zhangsenchuan <zhangsenchuan@...incomputing.com>
To: "Shawn Lin" <shawn.lin@...k-chips.com>
Cc: bhelgaas@...gle.com, mani@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, lpieralisi@...nel.org, kwilczynski@...nel.org,
robh@...nel.org, p.zabel@...gutronix.de, jingoohan1@...il.com,
gustavo.pimentel@...opsys.com, linux-pci@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
christian.bruel@...s.st.com, mayank.rana@....qualcomm.com,
shradha.t@...sung.com, krishna.chundru@....qualcomm.com,
thippeswamy.havalige@....com, inochiama@...il.com,
ningyu@...incomputing.com, linmin@...incomputing.com,
pinkesh.vaghela@...fochips.com, ouyanghui@...incomputing.com,
Frank.li@....com
Subject: Re: Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller
driver
> -----Original Messages-----
> From: "Shawn Lin" <shawn.lin@...k-chips.com>
> Send time:Thursday, 20/11/2025 21:19:40
> To: zhangsenchuan@...incomputing.com, bhelgaas@...gle.com, mani@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, lpieralisi@...nel.org, kwilczynski@...nel.org, robh@...nel.org, p.zabel@...gutronix.de, jingoohan1@...il.com, gustavo.pimentel@...opsys.com, linux-pci@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, christian.bruel@...s.st.com, mayank.rana@....qualcomm.com, shradha.t@...sung.com, krishna.chundru@....qualcomm.com, thippeswamy.havalige@....com, inochiama@...il.com
> Cc: shawn.lin@...k-chips.com, ningyu@...incomputing.com, linmin@...incomputing.com, pinkesh.vaghela@...fochips.com, ouyanghui@...incomputing.com, Frank.li@....com
> Subject: Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
>
> 在 2025/11/20 星期四 18:12, zhangsenchuan@...incomputing.com 写道:
> > From: Senchuan Zhang <zhangsenchuan@...incomputing.com>
> >
> > Add driver for the Eswin EIC7700 PCIe host controller, which is based on
> > the DesignWare PCIe core, IP revision 6.00a. The PCIe Gen.3 controller
> > supports a data rate of 8 GT/s and 4 channels, support INTx and MSI
> > interrupts.
> >
>
> Don't need any stuff regarding to PHY in the driver?
Thank you for your comment, Shawn
Clarification
The phy is automatically configured by the hardware, and there are processes
in the code waiting for the phy to be initialized.
>
> > Signed-off-by: Yu Ning <ningyu@...incomputing.com>
> > Signed-off-by: Yanghui Ou <ouyanghui@...incomputing.com>
> > Signed-off-by: Senchuan Zhang <zhangsenchuan@...incomputing.com>
> > ---
> > drivers/pci/controller/dwc/Kconfig | 11 +
> > drivers/pci/controller/dwc/Makefile | 1 +
> > drivers/pci/controller/dwc/pcie-eic7700.c | 387 ++++++++++++++++++++++
> > 3 files changed, 399 insertions(+)
> > create mode 100644 drivers/pci/controller/dwc/pcie-eic7700.c
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index 349d4657393c..66568efb324f 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -93,6 +93,17 @@ config PCIE_BT1
> > Enables support for the PCIe controller in the Baikal-T1 SoC to work
> > in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core.
> >
> > +config PCIE_EIC7700
> > + bool "Eswin EIC7700 PCIe controller"
> > + depends on ARCH_ESWIN || COMPILE_TEST
> > + depends on PCI_MSI
> > + select PCIE_DW_HOST
> > + help
> > + Say Y here if you want PCIe controller support for the Eswin EIC7700.
> > + The PCIe controller on EIC7700 is based on DesignWare hardware,
> > + enables support for the PCIe controller in the EIC7700 SoC to work in
> > + host mode.
> > +
> > config PCI_IMX6
> > bool
> >
> > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > index 7ae28f3b0fb3..04f751c49eba 100644
> > --- a/drivers/pci/controller/dwc/Makefile
> > +++ b/drivers/pci/controller/dwc/Makefile
> > @@ -6,6 +6,7 @@ obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> > obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> > obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
> > obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
> > +obj-$(CONFIG_PCIE_EIC7700) += pcie-eic7700.o
> > obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> > obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> > obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
> > diff --git a/drivers/pci/controller/dwc/pcie-eic7700.c b/drivers/pci/controller/dwc/pcie-eic7700.c
> > new file mode 100644
> > index 000000000000..239fdbc501fe
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-eic7700.c
> > @@ -0,0 +1,387 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * ESWIN EIC7700 PCIe root complex driver
> > + *
> > + * Copyright 2025, Beijing ESWIN Computing Technology Co., Ltd.
> > + *
> > + * Authors: Yu Ning <ningyu@...incomputing.com>
> > + * Senchuan Zhang <zhangsenchuan@...incomputing.com>
> > + * Yanghui Ou <ouyanghui@...incomputing.com>
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/resource.h>
> > +#include <linux/reset.h>
> > +#include <linux/types.h>
> > +
> > +#include "pcie-designware.h"
> > +
> > +/* ELBI registers */
> > +#define PCIEELBI_CTRL0_OFFSET 0x0
> > +#define PCIEELBI_STATUS0_OFFSET 0x100
> > +
> > +/* LTSSM register fields */
> > +#define PCIEELBI_APP_LTSSM_ENABLE BIT(5)
> > +
> > +/* APP_HOLD_PHY_RST register fields */
> > +#define PCIEELBI_APP_HOLD_PHY_RST BIT(6)
> > +
> > +/* PM_SEL_AUX_CLK register fields */
> > +#define PCIEELBI_PM_SEL_AUX_CLK BIT(16)
> > +
> > +/* DEV_TYPE register fields */
> > +#define PCIEELBI_CTRL0_DEV_TYPE GENMASK(3, 0)
> > +
> > +/* Vendor and device ID value */
> > +#define PCI_VENDOR_ID_ESWIN 0x1fe1
> > +#define PCI_DEVICE_ID_ESWIN 0x2030
>
> It would be better to be moved to pci_ids.h ?
No vendor using Synopsys IP was found in the pci_ids.h file to have
their VENDOR_ID and DEVICE_ID. Mani and bjorn may have agreed to keep
the VENDOR_ID and DEVICE_ID in our own file.
>
> > +
> > +#define EIC7700_NUM_RSTS ARRAY_SIZE(eic7700_pcie_rsts)
>
> If using devm_reset_control_array_get_exclusive, you don't need this
> at all.
The maintainer of the reset module, Philipp, recommended
devm_reset_control_bulk_get_exclusive() to me in the v4 patch. Maybe
he had other considerations.
>
> > +
> > +static const char * const eic7700_pcie_rsts[] = {
> > + "pwr",
> > + "dbi",
> > +};
> > +
>
> Ditto.
>
> > +struct eic7700_pcie_data {
> > + bool msix_cap;
> > + bool no_suspport_L2;
> > +};
> > +
> > +struct eic7700_pcie_port {
> > + struct list_head list;
> > + struct reset_control *perst;
> > + int num_lanes;
> > +};
> > +
> > +struct eic7700_pcie {
> > + struct dw_pcie pci;
> > + struct clk_bulk_data *clks;
> > + struct reset_control_bulk_data resets[EIC7700_NUM_RSTS];
> > + struct list_head ports;
> > + const struct eic7700_pcie_data *data;
> > + int num_clks;
> > +};
> > +
> > +#define to_eic7700_pcie(x) dev_get_drvdata((x)->dev)
> > +
> > +static int eic7700_pcie_start_link(struct dw_pcie *pci)
> > +{
> > + u32 val;
> > +
> > + /* Enable LTSSM */
> > + val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > + val |= PCIEELBI_APP_LTSSM_ENABLE;
> > + writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > +
> > + return 0;
> > +}
> > +
> > +static bool eic7700_pcie_link_up(struct dw_pcie *pci)
> > +{
> > + u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > + u16 val = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
>
> dw_pcie_readl_dbi()?
Okey, thanks:)
>
> > +
> > + return val & PCI_EXP_LNKSTA_DLLLA;
> > +}
> > +
> > +static int eic7700_pcie_perst_deassert(struct eic7700_pcie_port *port,
> > + struct eic7700_pcie *pcie)
> > +{
> > + int ret;
> > +
> > + ret = reset_control_assert(port->perst);
> > + if (ret) {
> > + dev_err(pcie->pci.dev, "Failed to assert PERST#\n");
> > + return ret;
> > + }
> > +
> > + /* Ensure that PERST# has been asserted for at least 100 ms */
> > + msleep(PCIE_T_PVPERL_MS);
> > +
> > + ret = reset_control_deassert(port->perst);
> > + if (ret) {
> > + dev_err(pcie->pci.dev, "Failed to deassert PERST#\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int eic7700_pcie_parse_port(struct eic7700_pcie *pcie,
> > + struct device_node *node)
> > +{
> > + struct device *dev = pcie->pci.dev;
> > + struct eic7700_pcie_port *port;
> > +
> > + port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> > + if (!port)
> > + return -ENOMEM;
> > +
> > + port->perst = of_reset_control_get_exclusive(node, "perst");
> > + if (IS_ERR(port->perst)) {
> > + dev_err(dev, "Failed to get PERST# reset\n");
> > + return PTR_ERR(port->perst);
> > + }
> > +
> > + /*
> > + * TODO: Since the Root Port node is separated out by pcie devicetree,
> > + * the DWC core initialization code can't parse the num-lanes attribute
> > + * in the Root Port. Before entering the DWC core initialization code,
> > + * the platform driver code parses the Root Port node. The EIC7700 only
> > + * supports one Root Port node, and the num-lanes attribute is suitable
> > + * for the case of one Root Rort.
> > + */
> > + if (!of_property_read_u32(node, "num-lanes", &port->num_lanes))
> > + pcie->pci.num_lanes = port->num_lanes;
> > +
>
> dw_pcie_get_resources() knows it.
The num-lanes of the root port node cannot be parsed in dw_pcie_get_resources.
Our device tree has separated the root port node.
Kind regards,
Senchuan Zhang
Powered by blists - more mailing lists