Message-ID: <f55e2df3fcd025148b7262c3753e5136cc324b09.camel@gmail.com>
Date: Mon, 17 Nov 2025 13:01:04 +0000
From: Vitor Soares <ivitro@...il.com>
To: Alex Elder <elder@...cstar.com>, lpieralisi@...nel.org, 
 kwilczynski@...nel.org, mani@...nel.org, robh@...nel.org,
 bhelgaas@...gle.com
Cc: dlan@...too.org, aurelien@...el32.net, johannes@...felt.com, 
 p.zabel@...gutronix.de, christian.bruel@...s.st.com, 
 thippeswamy.havalige@....com, krishna.chundru@....qualcomm.com, 
 mayank.rana@....qualcomm.com, qiang.yu@....qualcomm.com,
 shradha.t@...sung.com,  inochiama@...il.com, guodong@...cstar.com,
 linux-pci@...r.kernel.org,  spacemit@...ts.linux.dev,
 linux-riscv@...ts.infradead.org,  linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 5/7] PCI: spacemit: Add SpacemiT PCIe host driver

On Thu, 2025-11-13 at 15:45 -0600, Alex Elder wrote:
> Introduce a driver for the PCIe host controller found in the SpacemiT
> K1 SoC.  The hardware is derived from the Synopsys DesignWare PCIe IP.
> The driver supports three PCIe ports that operate at PCIe gen2 transfer
> rates (5 GT/sec).  The first port uses a combo PHY, which may be
> configured for use for USB 3 instead.
> 
> Signed-off-by: Alex Elder <elder@...cstar.com>
> ---
> v6: - Disabling ASPM L1 is now done earlier, at the end of the
>       dw_pcie_host_ops->init callback rather than ->post_init
>     - The function that disables ASPM L1 has been moved an renamed
>       to be k1_pcie_disable_aspm_l1()
>     - The return value from devm_platform_ioremap_resource_byname()
>       is now checked with IS_ERR()
>     - The number of MSI vectors implemented 256, rather than the default
>     - The sentinel entry in the OF match table no longer includes
>       a trailing comma
>     - MODULE_LICENSE() and MODULE_DESCRIPTION() macros are now
>       included
> 
>  drivers/pci/controller/dwc/Kconfig            |  13 +
>  drivers/pci/controller/dwc/Makefile           |   1 +
>  drivers/pci/controller/dwc/pcie-spacemit-k1.c | 358 ++++++++++++++++++
>  3 files changed, 372 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-spacemit-k1.c
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig
> b/drivers/pci/controller/dwc/Kconfig
> index 349d4657393c9..718bb54e943f6 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -416,6 +416,19 @@ config PCIE_SOPHGO_DW
>           Say Y here if you want PCIe host controller support on
>           Sophgo SoCs.
>  
> +config PCIE_SPACEMIT_K1
> +       tristate "SpacemiT K1 PCIe controller (host mode)"
> +       depends on ARCH_SPACEMIT || COMPILE_TEST
> +       depends on HAS_IOMEM
> +       select PCIE_DW_HOST
> +       select PCI_PWRCTRL_SLOT
> +       default ARCH_SPACEMIT
> +       help
> +         Enables support for the DesignWare based PCIe controller in
> +         the SpacemiT K1 SoC operating in host mode.  Three controllers
> +         are available on the K1 SoC; the first of these shares a PHY
> +         with a USB 3.0 host controller (one or the other can be used).
> +
>  config PCIE_SPEAR13XX
>         bool "STMicroelectronics SPEAr PCIe controller"
>         depends on ARCH_SPEAR13XX || COMPILE_TEST
> diff --git a/drivers/pci/controller/dwc/Makefile
> b/drivers/pci/controller/dwc/Makefile
> index 7ae28f3b0fb39..662b0a219ddc4 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
>  obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
>  obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
>  obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4.o
> +obj-$(CONFIG_PCIE_SPACEMIT_K1) += pcie-spacemit-k1.o
>  obj-$(CONFIG_PCIE_STM32_HOST) += pcie-stm32.o
>  obj-$(CONFIG_PCIE_STM32_EP) += pcie-stm32-ep.o
>  
> diff --git a/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> new file mode 100644
> index 0000000000000..451febfa326b7
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> @@ -0,0 +1,358 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SpacemiT K1 PCIe host driver
> + *
> + * Copyright (C) 2025 by RISCstar Solutions Corporation.  All rights
> reserved.
> + * Copyright (c) 2023, spacemit Corporation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gfp.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/types.h>
> +
> +#include "pcie-designware.h"
> +
> +#define PCI_VENDOR_ID_SPACEMIT         0x201f
> +#define PCI_DEVICE_ID_SPACEMIT_K1      0x0001
> +
> +/* Offsets and field definitions for link management registers */
> +#define K1_PHY_AHB_IRQ_EN                      0x0000
> +#define PCIE_INTERRUPT_EN              BIT(0)
> +
> +#define K1_PHY_AHB_LINK_STS                    0x0004
> +#define SMLH_LINK_UP                   BIT(1)
> +#define RDLH_LINK_UP                   BIT(12)
> +
> +#define INTR_ENABLE                            0x0014
> +#define MSI_CTRL_INT                   BIT(11)
> +
> +/* Some controls require APMU regmap access */
> +#define SYSCON_APMU                    "spacemit,apmu"
> +
> +/* Offsets and field definitions for APMU registers */
> +#define PCIE_CLK_RESET_CONTROL                 0x0000
> +#define LTSSM_EN                       BIT(6)
> +#define PCIE_AUX_PWR_DET               BIT(9)
> +#define PCIE_RC_PERST                  BIT(12) /* 1: assert PERST# */
> +#define APP_HOLD_PHY_RST               BIT(30)
> +#define DEVICE_TYPE_RC                 BIT(31) /* 0: endpoint; 1: RC */
> +
> +#define PCIE_CONTROL_LOGIC                     0x0004
> +#define PCIE_SOFT_RESET                        BIT(0)
> +
> +struct k1_pcie {
> +       struct dw_pcie pci;
> +       struct phy *phy;
> +       void __iomem *link;
> +       struct regmap *pmu;     /* Errors ignored; MMIO-backed regmap */
> +       u32 pmu_off;
> +};
> +
> +#define to_k1_pcie(dw_pcie) \
> +               platform_get_drvdata(to_platform_device((dw_pcie)->dev))
> +
> +static void k1_pcie_toggle_soft_reset(struct k1_pcie *k1)
> +{
> +       u32 offset;
> +       u32 val;
> +
> +       /*
> +        * Write, then read back to guarantee it has reached the device
> +        * before we start the delay.
> +        */
> +       offset = k1->pmu_off + PCIE_CONTROL_LOGIC;
> +       regmap_set_bits(k1->pmu, offset, PCIE_SOFT_RESET);
> +       regmap_read(k1->pmu, offset, &val);
> +
> +       mdelay(2);
> +
> +       regmap_clear_bits(k1->pmu, offset, PCIE_SOFT_RESET);
> +}
> +
> +/* Enable app clocks, deassert resets */
> +static int k1_pcie_enable_resources(struct k1_pcie *k1)
> +{
> +       struct dw_pcie *pci = &k1->pci;
> +       int ret;
> +
> +       ret = clk_bulk_prepare_enable(ARRAY_SIZE(pci->app_clks), pci-
> >app_clks);
> +       if (ret)
> +               return ret;
> +
> +       ret = reset_control_bulk_deassert(ARRAY_SIZE(pci->app_rsts),
> +                                         pci->app_rsts);
> +       if (ret)
> +               goto err_disable_clks;
> +
> +       return 0;
> +
> +err_disable_clks:
> +       clk_bulk_disable_unprepare(ARRAY_SIZE(pci->app_clks), pci->app_clks);
> +
> +       return ret;
> +}
> +
> +/* Assert resets, disable app clocks */
> +static void k1_pcie_disable_resources(struct k1_pcie *k1)
> +{
> +       struct dw_pcie *pci = &k1->pci;
> +
> +       reset_control_bulk_assert(ARRAY_SIZE(pci->app_rsts), pci->app_rsts);
> +       clk_bulk_disable_unprepare(ARRAY_SIZE(pci->app_clks), pci->app_clks);
> +}
> +
> +/* Disable ASPM L1 to avoid errors reported on some NVMe drives */
> +static void k1_pcie_disable_aspm_l1(struct k1_pcie *k1)
> +{
> +       struct dw_pcie *pci = &k1->pci;
> +       u8 offset;
> +       u32 val;
> +
> +       offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +       offset += PCI_EXP_LNKCAP;
> +
> +       /* Turn off ASPM L1 for the link */
> +       dw_pcie_dbi_ro_wr_en(pci);
> +       val = dw_pcie_readl_dbi(pci, offset);
> +       val &= ~PCI_EXP_LNKCAP_ASPM_L1;
> +       dw_pcie_writel_dbi(pci, offset, val);
> +       dw_pcie_dbi_ro_wr_dis(pci);
> +}
> +
> +static int k1_pcie_init(struct dw_pcie_rp *pp)
> +{
> +       struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +       struct k1_pcie *k1 = to_k1_pcie(pci);
> +       u32 reset_ctrl;
> +       u32 val;
> +       int ret;
> +
> +       k1_pcie_toggle_soft_reset(k1);
> +
> +       ret = k1_pcie_enable_resources(k1);
> +       if (ret)
> +               return ret;
> +
> +       /* Set the PCI vendor and device ID */
> +       dw_pcie_dbi_ro_wr_en(pci);
> +       dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, PCI_VENDOR_ID_SPACEMIT);
> +       dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, PCI_DEVICE_ID_SPACEMIT_K1);
> +       dw_pcie_dbi_ro_wr_dis(pci);
> +
> +       /*
> +        * Start by asserting fundamental reset (drive PERST# low).  The
> +        * PCI CEM spec says that PERST# should be deasserted at least
> +        * 100ms after the power becomes stable, so we'll insert that
> +        * delay first.  Write, then read it back to guarantee the write
> +        * reaches the device before we start the delay.
> +        */
> +       reset_ctrl = k1->pmu_off + PCIE_CLK_RESET_CONTROL;
> +       regmap_set_bits(k1->pmu, reset_ctrl, PCIE_RC_PERST);
> +       regmap_read(k1->pmu, reset_ctrl, &val);
> +       mdelay(PCIE_T_PVPERL_MS);
> +
> +       /*
> +        * Put the controller in root complex mode, and indicate that
> +        * Vaux (3.3v) is present.
> +        */
> +       regmap_set_bits(k1->pmu, reset_ctrl, DEVICE_TYPE_RC |
> PCIE_AUX_PWR_DET);
> +
> +       ret = phy_init(k1->phy);
> +       if (ret) {
> +               k1_pcie_disable_resources(k1);
> +
> +               return ret;
> +       }
> +
> +       /* Deassert fundamental reset (drive PERST# high) */
> +       regmap_clear_bits(k1->pmu, reset_ctrl, PCIE_RC_PERST);
> +
> +       /* Finally, as a workaround, disable ASPM L1 */
> +       k1_pcie_disable_aspm_l1(k1);
> +
> +       return 0;
> +}
> +
> +static void k1_pcie_deinit(struct dw_pcie_rp *pp)
> +{
> +       struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +       struct k1_pcie *k1 = to_k1_pcie(pci);
> +
> +       /* Assert fundamental reset (drive PERST# low) */
> +       regmap_set_bits(k1->pmu, k1->pmu_off + PCIE_CLK_RESET_CONTROL,
> +                       PCIE_RC_PERST);
> +
> +       phy_exit(k1->phy);
> +
> +       k1_pcie_disable_resources(k1);
> +}
> +
> +static const struct dw_pcie_host_ops k1_pcie_host_ops = {
> +       .init           = k1_pcie_init,
> +       .deinit         = k1_pcie_deinit,
> +};
> +
> +static bool k1_pcie_link_up(struct dw_pcie *pci)
> +{
> +       struct k1_pcie *k1 = to_k1_pcie(pci);
> +       u32 val;
> +
> +       val = readl_relaxed(k1->link + K1_PHY_AHB_LINK_STS);
> +
> +       return (val & RDLH_LINK_UP) && (val & SMLH_LINK_UP);
> +}
> +
> +static int k1_pcie_start_link(struct dw_pcie *pci)
> +{
> +       struct k1_pcie *k1 = to_k1_pcie(pci);
> +       u32 val;
> +
> +       /* Stop holding the PHY in reset, and enable link training */
> +       regmap_update_bits(k1->pmu, k1->pmu_off + PCIE_CLK_RESET_CONTROL,
> +                          APP_HOLD_PHY_RST | LTSSM_EN, LTSSM_EN);
> +
> +       /* Enable the MSI interrupt */
> +       writel_relaxed(MSI_CTRL_INT, k1->link + INTR_ENABLE);
> +
> +       /* Top-level interrupt enable */
> +       val = readl_relaxed(k1->link + K1_PHY_AHB_IRQ_EN);
> +       val |= PCIE_INTERRUPT_EN;
> +       writel_relaxed(val, k1->link + K1_PHY_AHB_IRQ_EN);
> +
> +       return 0;
> +}
> +
> +static void k1_pcie_stop_link(struct dw_pcie *pci)
> +{
> +       struct k1_pcie *k1 = to_k1_pcie(pci);
> +       u32 val;
> +
> +       /* Disable interrupts */
> +       val = readl_relaxed(k1->link + K1_PHY_AHB_IRQ_EN);
> +       val &= ~PCIE_INTERRUPT_EN;
> +       writel_relaxed(val, k1->link + K1_PHY_AHB_IRQ_EN);
> +
> +       writel_relaxed(0, k1->link + INTR_ENABLE);
> +
> +       /* Disable the link and hold the PHY in reset */
> +       regmap_update_bits(k1->pmu, k1->pmu_off + PCIE_CLK_RESET_CONTROL,
> +                          APP_HOLD_PHY_RST | LTSSM_EN, APP_HOLD_PHY_RST);
> +}
> +
> +static const struct dw_pcie_ops k1_pcie_ops = {
> +       .link_up        = k1_pcie_link_up,
> +       .start_link     = k1_pcie_start_link,
> +       .stop_link      = k1_pcie_stop_link,
> +};
> +
> +static int k1_pcie_parse_port(struct k1_pcie *k1)
> +{
> +       struct device *dev = k1->pci.dev;
> +       struct device_node *root_port;
> +       struct phy *phy;
> +
> +       /* We assume only one root port */
> +       root_port = of_get_next_available_child(dev_of_node(dev), NULL);
> +       if (!root_port)
> +               return -EINVAL;
> +
> +       phy = devm_of_phy_get(dev, root_port, NULL);
> +
> +       of_node_put(root_port);
> +
> +       if (IS_ERR(phy))
> +               return PTR_ERR(phy);
> +
> +       k1->phy = phy;
> +
> +       return 0;
> +}
> +
> +static int k1_pcie_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct k1_pcie *k1;
> +       int ret;
> +
> +       k1 = devm_kzalloc(dev, sizeof(*k1), GFP_KERNEL);
> +       if (!k1)
> +               return -ENOMEM;
> +
> +       k1->pmu = syscon_regmap_lookup_by_phandle_args(dev_of_node(dev),
> +                                                      SYSCON_APMU, 1,
> +                                                      &k1->pmu_off);
> +       if (IS_ERR(k1->pmu))
> +               return dev_err_probe(dev, PTR_ERR(k1->pmu),
> +                                    "failed to lookup PMU registers\n");
> +
> +       k1->link = devm_platform_ioremap_resource_byname(pdev, "link");
> +       if (IS_ERR(k1->link))
> +               return dev_err_probe(dev, PTR_ERR(k1->link),
> +                                    "failed to map \"link\" registers\n");
> +
> +       k1->pci.dev = dev;
> +       k1->pci.ops = &k1_pcie_ops;
> +       k1->pci.pp.num_vectors = MAX_MSI_IRQS;
> +       dw_pcie_cap_set(&k1->pci, REQ_RES);
> +
> +       k1->pci.pp.ops = &k1_pcie_host_ops;
> +
> +       /* Hold the PHY in reset until we start the link */
> +       regmap_set_bits(k1->pmu, k1->pmu_off + PCIE_CLK_RESET_CONTROL,
> +                       APP_HOLD_PHY_RST);
> +
> +       ret = devm_regulator_get_enable(dev, "vpcie3v3");
> +       if (ret)
> +               return dev_err_probe(dev, ret,
> +                                    "failed to get \"vpcie3v3\" supply\n");
> +

Hi,

While investigation a similar PCIe setup, I come across this patch series and
had a question regarding regulator handling.

In the patch, the host controller explicitly enables the vpcie3v3. However, the
pci-pwctrl-slot driver (compatible pciclass,0604) is also instantiated via
device tree and is supposed to manage the slot supply.

I'm wondering whether it's actually necessary to enable the regulator in the
host controller, since the slot driver should already take care of it. However,
in my tests, the endpoint is not discovered unless the host driver enables the
regulator early. It's not entirely clear whether this is due to the PHY and
reset sequence, or the timing of the vpcie3v3 regulator activation.

@Manivannan: In the patch, the host controller driver enables the vpcie3v3
regulator, even though the pci-pwrctrl-slot driver (pciclass,0604) is supposed
to manage it. Is this duplication intentional, or is there a preferred way to
ensure the supply is active before bus scanning? Any guidance would be
appreciated.


Thanks in advance!
Vitor Soares

Powered by blists - more mailing lists