[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <kd3wxvditosgj7rihh2q5iqvl43ljunbxaqbqpcxpmsbdnsbga@f4jm3o33ilkv>
Date: Fri, 12 Sep 2025 10:47:43 +0800
From: Inochi Amaoto <inochiama@...il.com>
To: Manivannan Sadhasivam <mani@...nel.org>,
Inochi Amaoto <inochiama@...il.com>
Cc: Chen Wang <unicornxw@...il.com>, kwilczynski@...nel.org,
u.kleine-koenig@...libre.com, aou@...s.berkeley.edu, alex@...ti.fr, arnd@...db.de,
bwawrzyn@...co.com, bhelgaas@...gle.com, unicorn_wang@...look.com,
conor+dt@...nel.org, 18255117159@....com, kishon@...nel.org, krzk+dt@...nel.org,
lpieralisi@...nel.org, palmer@...belt.com, paul.walmsley@...ive.com, robh@...nel.org,
s-vadapalli@...com, tglx@...utronix.de, thomas.richard@...tlin.com,
sycamoremoon376@...il.com, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pci@...r.kernel.org, linux-riscv@...ts.infradead.org, sophgo@...ts.linux.dev,
rabenda.cn@...il.com, chao.wei@...hgo.com, xiaoguang.xing@...hgo.com,
fengchun.li@...hgo.com
Subject: Re: [PATCH v2 3/7] PCI: sg2042: Add Sophgo SG2042 PCIe driver
On Thu, Sep 11, 2025 at 10:33:18PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Sep 10, 2025 at 10:56:23AM GMT, Inochi Amaoto wrote:
> > On Wed, Sep 10, 2025 at 10:08:39AM +0800, Chen Wang wrote:
> > > From: Chen Wang <unicorn_wang@...look.com>
> > >
> > > Add support for PCIe controller in SG2042 SoC. The controller
> > > uses the Cadence PCIe core programmed by pcie-cadence*.c. The
> > > PCIe controller will work in host mode only, supporting data
> > > rate(gen4) and lanes(x16 or x8).
> > >
> > > Signed-off-by: Chen Wang <unicorn_wang@...look.com>
> > > ---
> > > drivers/pci/controller/cadence/Kconfig | 10 ++
> > > drivers/pci/controller/cadence/Makefile | 1 +
> > > drivers/pci/controller/cadence/pcie-sg2042.c | 104 +++++++++++++++++++
> > > 3 files changed, 115 insertions(+)
> > > create mode 100644 drivers/pci/controller/cadence/pcie-sg2042.c
> > >
> > > diff --git a/drivers/pci/controller/cadence/Kconfig b/drivers/pci/controller/cadence/Kconfig
> > > index 666e16b6367f..02a639e55fd8 100644
> > > --- a/drivers/pci/controller/cadence/Kconfig
> > > +++ b/drivers/pci/controller/cadence/Kconfig
> > > @@ -42,6 +42,15 @@ config PCIE_CADENCE_PLAT_EP
> > > endpoint mode. This PCIe controller may be embedded into many
> > > different vendors SoCs.
> > >
> > > +config PCIE_SG2042_HOST
> > > + tristate "Sophgo SG2042 PCIe controller (host mode)"
> > > + depends on OF && (ARCH_SOPHGO || COMPILE_TEST)
> > > + select PCIE_CADENCE_HOST
> > > + help
> > > + Say Y here if you want to support the Sophgo SG2042 PCIe platform
> > > + controller in host mode. Sophgo SG2042 PCIe controller uses Cadence
> > > + PCIe core.
> > > +
> > > config PCI_J721E
> > > tristate
> > > select PCIE_CADENCE_HOST if PCI_J721E_HOST != n
> > > @@ -67,4 +76,5 @@ config PCI_J721E_EP
> > > Say Y here if you want to support the TI J721E PCIe platform
> > > controller in endpoint mode. TI J721E PCIe controller uses Cadence PCIe
> > > core.
> > > +
> > > endmenu
> > > diff --git a/drivers/pci/controller/cadence/Makefile b/drivers/pci/controller/cadence/Makefile
> > > index 9bac5fb2f13d..5e23f8539ecc 100644
> > > --- a/drivers/pci/controller/cadence/Makefile
> > > +++ b/drivers/pci/controller/cadence/Makefile
> > > @@ -4,3 +4,4 @@ obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o
> > > obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o
> > > obj-$(CONFIG_PCIE_CADENCE_PLAT) += pcie-cadence-plat.o
> > > obj-$(CONFIG_PCI_J721E) += pci-j721e.o
> > > +obj-$(CONFIG_PCIE_SG2042_HOST) += pcie-sg2042.o
> > > diff --git a/drivers/pci/controller/cadence/pcie-sg2042.c b/drivers/pci/controller/cadence/pcie-sg2042.c
> > > new file mode 100644
> > > index 000000000000..c026e1ca5d6e
> > > --- /dev/null
> > > +++ b/drivers/pci/controller/cadence/pcie-sg2042.c
> > > @@ -0,0 +1,104 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * pcie-sg2042 - PCIe controller driver for Sophgo SG2042 SoC
> > > + *
> > > + * Copyright (C) 2025 Sophgo Technology Inc.
> > > + * Copyright (C) 2025 Chen Wang <unicorn_wang@...look.com>
> > > + */
> > > +
> > > +#include <linux/mod_devicetable.h>
> > > +#include <linux/pci.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/pm_runtime.h>
> > > +
> > > +#include "pcie-cadence.h"
> > > +
> > > +/*
> > > + * SG2042 only supports 4-byte aligned access, so for the rootbus (i.e. to
> > > + * read/write the Root Port itself, read32/write32 is required. For
> > > + * non-rootbus (i.e. to read/write the PCIe peripheral registers, supports
> > > + * 1/2/4 byte aligned access, so directly using read/write should be fine.
> > > + */
> > > +
> > > +static struct pci_ops sg2042_pcie_root_ops = {
> > > + .map_bus = cdns_pci_map_bus,
> > > + .read = pci_generic_config_read32,
> > > + .write = pci_generic_config_write32,
> > > +};
> > > +
> > > +static struct pci_ops sg2042_pcie_child_ops = {
> > > + .map_bus = cdns_pci_map_bus,
> > > + .read = pci_generic_config_read,
> > > + .write = pci_generic_config_write,
> > > +};
> > > +
> > > +static int sg2042_pcie_probe(struct platform_device *pdev)
> > > +{
> > > + struct device *dev = &pdev->dev;
> > > + struct pci_host_bridge *bridge;
> > > + struct cdns_pcie *pcie;
> > > + struct cdns_pcie_rc *rc;
> > > + int ret;
> > > +
> > > + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc));
> > > + if (!bridge) {
> > > + dev_err_probe(dev, -ENOMEM, "Failed to alloc host bridge!\n");
> > > + return -ENOMEM;
> > > + }
> > > +
> > > + bridge->ops = &sg2042_pcie_root_ops;
> > > + bridge->child_ops = &sg2042_pcie_child_ops;
> > > +
> > > + rc = pci_host_bridge_priv(bridge);
> > > + pcie = &rc->pcie;
> > > + pcie->dev = dev;
> > > +
> > > + platform_set_drvdata(pdev, pcie);
> > > +
> > > + pm_runtime_set_active(dev);
> > > + pm_runtime_no_callbacks(dev);
> > > + devm_pm_runtime_enable(dev);
> > > +
> > > + ret = cdns_pcie_init_phy(dev, pcie);
> > > + if (ret) {
> > > + dev_err_probe(dev, ret, "Failed to init phy!\n");
> > > + return ret;
> > > + }
> > > +
> > > + ret = cdns_pcie_host_setup(rc);
> > > + if (ret) {
> > > + dev_err_probe(dev, ret, "Failed to setup host!\n");
> > > + cdns_pcie_disable_phy(pcie);
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> >
> > > +static void sg2042_pcie_remove(struct platform_device *pdev)
> > > +{
> > > + struct cdns_pcie *pcie = platform_get_drvdata(pdev);
> > > +
> > > + cdns_pcie_disable_phy(pcie);
> > > +}
> > > +
> >
> > I think this remove is useless, as it is almost impossible to
> > remove a pcie at runtime.
> >
>
> Why impossible? We only have concerns with removing PCIe controllers
> implementing irqchip, but this driver is not implementing it and using an
> external irqchip controller.
>
> So it is safe and possible to remove this driver during runtime.
>
Good to know this. It is the thing I did not know before.
So it is OK for me to see this code.
Thanks,
Inochi
Powered by blists - more mailing lists