Re: [PATCH v7 6/6] PCI: spacemit-k1: Add Spacemit K3 PCIe host controller support

[Siddharth Vadapalli <s-vadapalli@ti.com>]

On 10/21/26 1:26 PM, Inochi Amaoto wrote:
> On Sun, Oct 18, 2026 at 06:05:39PM +0530, Manivannan Sadhasivam wrote:
>> On 2026-09-29 16:37 +0800, Inochi Amaoto wrote:
>>> The PCIe controller on Spacemit K3 is almost a standard Synopsys
>>> DesignWare PCIe IP with extra link and reset control. Unlike
>>> the PCIe controller on K1, this controller supports external MSI
>>> interrupt controller and can use multiple PHYs at the same time.
>>>
>>> Add driver to support PCIe controller on Spacemit K3 PCIe.
>>>
>>> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
>>> ---
>>>   drivers/pci/controller/dwc/Kconfig            |   4 +-
>>>   drivers/pci/controller/dwc/pcie-spacemit-k1.c | 139 ++++++++++++++++++
>>>   2 files changed, 141 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>>> index dcfbe7e229fd..f9c27165260e 100644
>>> --- a/drivers/pci/controller/dwc/Kconfig
>>> +++ b/drivers/pci/controller/dwc/Kconfig
>>> @@ -443,7 +443,7 @@ config PCIE_SOPHGO_DW
>>>   	  Sophgo SoCs.
>>>   
>>>   config PCIE_SPACEMIT_K1
>>> -	tristate "SpacemiT K1 PCIe controller (host mode)"
>>> +	tristate "SpacemiT PCIe controller (host mode)"
>>>   	depends on ARCH_SPACEMIT || COMPILE_TEST
>>>   	depends on HAS_IOMEM
>>>   	select PCIE_DW_HOST
>>> @@ -451,7 +451,7 @@ config PCIE_SPACEMIT_K1
>>>   	default ARCH_SPACEMIT
>>>   	help
>>>   	  Enables support for the DesignWare based PCIe controller in
>>> -	  the SpacemiT K1 SoC operating in host mode.  Three controllers
>>> +	  the SpacemiT 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).
>>>   
>>> diff --git a/drivers/pci/controller/dwc/pcie-spacemit-k1.c b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
>>> index ec59ccb710ec..61b430f2294a 100644
>>> --- a/drivers/pci/controller/dwc/pcie-spacemit-k1.c
>>> +++ b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
>>> @@ -28,8 +28,18 @@
>>>   #define SMLH_LINK_UP			BIT(1)
>>>   #define RDLH_LINK_UP			BIT(12)
>>>   
>>> +#define INTR_STATUS				0x0010
>>> +
>>>   #define INTR_ENABLE				0x0014
>>>   #define MSI_CTRL_INT			BIT(11)
>>> +#define RDLH_LINK_UP_INT		BIT(20)
>>> +
>>> +#define K3_PHY_AHB_IRQSTATUS_INTX		0x0008
>>> +
>>> +#define K3_ADDR_INTR_STATUS1			0x0018
>>> +
>>> +#define K3_CACHE_MSTR_AWCACHE_MODE	GENMASK(14, 11)
>>> +#define K3_CACHE_MSTR_AWCACHE_BEHAVIOR	0xf
>>>   
>>>   /* Some controls require APMU regmap access */
>>>   #define SYSCON_APMU			"spacemit,apmu"
>>> @@ -44,6 +54,9 @@
>>>   
>>>   #define PCIE_CONTROL_LOGIC			0x0004
>>>   #define PCIE_SOFT_RESET			BIT(0)
>>> +#define PCIE_PERSTN_OE			BIT(24)
>>> +#define PCIE_PERSTN_OUT			BIT(25)
>>> +#define PCIE_IGNORE_PERSTN		BIT(31)
>>>   
>>>   struct k1_pcie;
>>>   
>>> @@ -315,6 +328,123 @@ static int k1_pcie_parse_port(struct k1_pcie *k1)
>>>   	return ret;
>>>   }
>>>   
>>> +static int k3_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 = k1->pmu_off + PCIE_CLK_RESET_CONTROL;
>>> +	u32 val;
>>> +	int ret;
>>> +
>>> +	regmap_clear_bits(k1->pmu, reset_ctrl, LTSSM_EN);
>>> +
>>> +	k1_pcie_toggle_soft_reset(k1);
>>> +
>>> +	/* K3: Set IGNORE_PERSTN and drive PERSTN_OE high (assert reset) */
>>> +	regmap_update_bits(k1->pmu, k1->pmu_off + PCIE_CONTROL_LOGIC,
>>> +			   PCIE_IGNORE_PERSTN | PCIE_PERSTN_OE | PCIE_PERSTN_OUT,
>>> +			   PCIE_IGNORE_PERSTN | PCIE_PERSTN_OE);
>>> +
>>> +	ret = k1_pcie_enable_resources(k1);
>>> +	if (ret)
>>> +		goto failed_resources;
>>> +
>>> +	regmap_set_bits(k1->pmu, reset_ctrl, PCIE_AUX_PWR_DET);
>>> +	regmap_clear_bits(k1->pmu, reset_ctrl, APP_HOLD_PHY_RST);
>>> +
>>> +	ret = phy_bulk_init(k1->phy_count, k1->phys);
>>> +	if (ret)
>>> +		goto failed_phy_init;
>>
>> You got me confused with this PHY bulk API for a moment. I think you are trying
>> to control all the PHYs of the multi-port RC setup at once. But I believe
>> that's not what we want mostly. Each port is an independent entity with its own
>> PHY, PERST# etc... So the RC driver controlling all the PHYs at once, doesn't
>> seem logical.
>>
> 
> In fact this thing is different. Accoring to statement from spacemit,
> for software, it does not need to care about how the phy topology,
> and which phy/phys is/are attached to the RC. It just needs to enable
> all the phy and configure the APMU, which is the one that give some
> control on PERST# and others. The phy itself does nothing on this.
According to section "14.1.4 PHY Configuration Block Diagram" in the User 
Manual for the Spacemit K3 SoC [0],the x8 Lane PCIe Controller namely 'PCIe 
A' requires multiple PHYs (ranging from PHY0 to PHY5) to be 
initialized/enabled simultaneously for a single Root-Port configuration. So 
the phy_bulk_init() API here seems to be used in that sense. Mani's concern 
is that the PHYs for all PCIe Controller instances are being configured in 
a single invocation to phy_bulk_init() rather than the potentially multiple 
PHY instances of a single PCIe Controller instance. Since k3_pcie_init is 
invoked per Root-Port instance (i.e. per PCIe Controller instance), the 
phy_bulk_init() API should configure 'all' PHY instances _strictly_ 
associated with the corresponding PCIe instance. This might already be the 
case.

[0]: 
https://spacemit.com/community/document/info?lang=en&nodepath=hardware/key_stone/k3/k3_docs/k3_usermanual/14_connectivity/pcie.md