[PATCH 0/5] riscv: spacemit: Add PCIe RC controller support for K3

[PATCH 0/5] riscv: spacemit: Add PCIe RC controller support for K3

[Inochi Amaoto]

The PCIe controller on Spacemit K3 is almost a standard Synopsys
Designware PCIe IP with extra control and external MSI controller
(IMSIC).

Add binding and driver support for PCIe RC controller support on K3.

Inochi Amaoto (5):
  PCI: spacemit-k1: Add device data support
  PCI: spacemit-k1: Add multiple phy handles support
  dt-bindings: PCI: snps,dw-pcie: Add msi-parent for msi handle check
  dt-bindings: pci: spacemit: Introduce Spacemit K3 PCIe host controller
  PCI: spacemit-k1: Add Spacemit K3 PCIe host controller support

 .../devicetree/bindings/pci/snps,dw-pcie.yaml |   7 +-
 .../bindings/pci/spacemit,k3-pcie-host.yaml   | 142 +++++++++
 drivers/pci/controller/dwc/pcie-spacemit-k1.c | 289 +++++++++++++++++-
 3 files changed, 428 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml

--
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 1/5] PCI: spacemit-k1: Add device data support

[Inochi Amaoto]

To reuse the K1 PCIe driver logic for K3 PCIe controller, add device
data to handle the K1 specific logic and make room for the incoming
logic for K3.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 drivers/pci/controller/dwc/pcie-spacemit-k1.c | 35 ++++++++++++++++---
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-spacemit-k1.c b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
index be20a520255b..cd3cd038ad2b 100644
--- a/drivers/pci/controller/dwc/pcie-spacemit-k1.c
+++ b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
@@ -57,6 +57,13 @@ struct k1_pcie {
 	u32 pmu_off;
 };
 
+struct k1_pcie_device_data {
+	const struct dw_pcie_host_ops *host_ops;
+	const struct dw_pcie_ops *ops;
+	int (*parse_port)(struct k1_pcie *k1);
+	int (*post_init)(struct k1_pcie *k1);
+};
+
 #define to_k1_pcie(dw_pcie) \
 		platform_get_drvdata(to_platform_device((dw_pcie)->dev))
 
@@ -278,10 +285,15 @@ static int k1_pcie_parse_port(struct k1_pcie *k1)
 
 static int k1_pcie_probe(struct platform_device *pdev)
 {
+	const struct k1_pcie_device_data *data;
 	struct device *dev = &pdev->dev;
 	struct k1_pcie *k1;
 	int ret;
 
+	data = device_get_match_data(dev);
+	if (!data)
+		return -ENODEV;
+
 	k1 = devm_kzalloc(dev, sizeof(*k1), GFP_KERNEL);
 	if (!k1)
 		return -ENOMEM;
@@ -299,11 +311,11 @@ static int k1_pcie_probe(struct platform_device *pdev)
 				     "failed to map \"link\" registers\n");
 
 	k1->pci.dev = dev;
-	k1->pci.ops = &k1_pcie_ops;
+	k1->pci.ops = data->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;
+	k1->pci.pp.ops = data->host_ops;
 
 	/* Hold the PHY in reset until we start the link */
 	regmap_set_bits(k1->pmu, k1->pmu_off + PCIE_CLK_RESET_CONTROL,
@@ -320,7 +332,7 @@ static int k1_pcie_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, k1);
 
-	ret = k1_pcie_parse_port(k1);
+	ret = data->parse_port(k1);
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to parse root port\n");
 
@@ -328,6 +340,15 @@ static int k1_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to initialize host\n");
 
+	if (data->post_init) {
+		ret = data->post_init(k1);
+		if (ret) {
+			dw_pcie_host_deinit(&k1->pci.pp);
+			return dev_err_probe(dev, ret,
+					     "Failed to post init\n");
+		}
+	}
+
 	return 0;
 }
 
@@ -338,8 +359,14 @@ static void k1_pcie_remove(struct platform_device *pdev)
 	dw_pcie_host_deinit(&k1->pci.pp);
 }
 
+static const struct k1_pcie_device_data k1_pcie_device_data = {
+	.host_ops	= &k1_pcie_host_ops,
+	.ops		= &k1_pcie_ops,
+	.parse_port	= k1_pcie_parse_port,
+};
+
 static const struct of_device_id k1_pcie_of_match_table[] = {
-	{ .compatible = "spacemit,k1-pcie", },
+	{ .compatible = "spacemit,k1-pcie", .data = &k1_pcie_device_data},
 	{ }
 };
 
-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/5] PCI: spacemit-k1: Add multiple phy handles support

[Inochi Amaoto]

The PCIe controller on Spacemit K3 may use multiple phys at the
same time. The feature is not support by the current driver.
So extend the phy definition to support multiple phy handles.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 drivers/pci/controller/dwc/pcie-spacemit-k1.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-spacemit-k1.c b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
index cd3cd038ad2b..f2a722e5edb5 100644
--- a/drivers/pci/controller/dwc/pcie-spacemit-k1.c
+++ b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
@@ -51,7 +51,8 @@
 
 struct k1_pcie {
 	struct dw_pcie pci;
-	struct phy *phy;
+	struct phy **phy;
+	int phy_count;
 	void __iomem *link;
 	struct regmap *pmu;	/* Errors ignored; MMIO-backed regmap */
 	u32 pmu_off;
@@ -172,7 +173,7 @@ static int k1_pcie_init(struct dw_pcie_rp *pp)
 	 */
 	regmap_set_bits(k1->pmu, reset_ctrl, DEVICE_TYPE_RC | PCIE_AUX_PWR_DET);
 
-	ret = phy_init(k1->phy);
+	ret = phy_init(k1->phy[0]);
 	if (ret) {
 		k1_pcie_disable_resources(k1);
 
@@ -192,12 +193,14 @@ 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);
+	int i;
 
 	/* 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);
+	for (i = 0; i < k1->phy_count; i++)
+		phy_exit(k1->phy[i]);
 
 	k1_pcie_disable_resources(k1);
 }
@@ -278,7 +281,12 @@ static int k1_pcie_parse_port(struct k1_pcie *k1)
 	if (IS_ERR(phy))
 		return PTR_ERR(phy);
 
-	k1->phy = phy;
+	k1->phy = devm_kmalloc_array(dev, sizeof(*k1->phy), 1, GFP_KERNEL);
+	if (IS_ERR(k1->phy))
+		return PTR_ERR(k1->phy);
+
+	k1->phy[0] = phy;
+	k1->phy_count = 1;
 
 	return 0;
 }
-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 3/5] dt-bindings: PCI: snps,dw-pcie: Add msi-parent for msi handle check

[Inochi Amaoto]

The IMSIC device on RISC-V based system does not require ID
remapping for MSI. So this device only needs "msi-parent"
property for IMSIC-based SoC, and the "msi-map" is not a
necessary property.

Add new condition for msi handling on IMSIC based SoC.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index b3216141881c..6a595207fae1 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -27,8 +27,11 @@ allOf:
   - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
   - if:
       not:
-        required:
-          - msi-map
+        oneOf:
+          - required:
+              - msi-map
+          - required:
+              - msi-parent
     then:
       properties:
         interrupt-names:
-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 4/5] dt-bindings: pci: spacemit: Introduce Spacemit K3 PCIe host controller

[Inochi Amaoto]

Add binding support for the PCIe controller on the SpacemiT K3 SoC.
This controller is almost a standard Synopsys Designware PCIe IP,
with some extra link and reset state control.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 .../bindings/pci/spacemit,k3-pcie-host.yaml   | 142 ++++++++++++++++++
 1 file changed, 142 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml

diff --git a/Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml b/Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml
new file mode 100644
index 000000000000..be2641526b19
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml
@@ -0,0 +1,142 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/spacemit,k3-pcie-host.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SpacemiT K3 PCI Express Host Controller
+
+maintainers:
+  - Inochi Amaoto <inochiama@gmail.com>
+
+description:
+  The SpacemiT K3 SoC PCIe host controller is based on the Synopsys
+  DesignWare PCIe IP.  The controller uses the external MSI interrupt
+  controller.
+
+allOf:
+  - $ref: /schemas/pci/pci-host-bridge.yaml#
+  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+
+properties:
+  compatible:
+    const: spacemit,k3-pcie
+
+  reg:
+    items:
+      - description: DesignWare PCIe registers
+      - description: Data Bus Interface (DBI) shadow registers
+      - description: ATU address space
+      - description: PCIe configuration space
+      - description: Link control registers
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: dbi2
+      - const: atu
+      - const: config
+      - const: link
+
+  clocks:
+    items:
+      - description: DWC PCIe Data Bus Interface (DBI) clock
+      - description: DWC PCIe application AXI-bus master interface clock
+      - description: DWC PCIe application AXI-bus slave interface clock
+
+  clock-names:
+    items:
+      - const: dbi
+      - const: mstr
+      - const: slv
+
+  resets:
+    items:
+      - description: DWC PCIe Data Bus Interface (DBI) reset
+      - description: DWC PCIe application AXI-bus master interface reset
+      - description: DWC PCIe application AXI-bus slave interface reset
+
+  reset-names:
+    items:
+      - const: dbi
+      - const: mstr
+      - const: slv
+
+  interrupts:
+    items:
+      - description: Interrupt used for port state
+
+  interrupt-names:
+    const: app
+
+  msi-parent: true
+
+  phys:
+    minItems: 1
+    maxItems: 6
+
+  phy-names:
+    minItems: 1
+    maxItems: 6
+
+  spacemit,apmu:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      A phandle that refers to the APMU system controller, whose regmap is
+      used in managing resets and link state, along with and offset of its
+      reset control register.
+    items:
+      - items:
+          - description: phandle to APMU system controller
+          - description: register offset
+
+required:
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - interrupts
+  - interrupt-names
+  - msi-parent
+  - spacemit,apmu
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      pcie@80000000 {
+        compatible = "spacemit,k3-pcie";
+        reg = <0x0  0x80000000 0x0 0x00001000>,
+              <0x0  0x80100000 0x0 0x00001000>,
+              <0x0  0x80300000 0x0 0x00003f20>,
+              <0x11 0x00000000 0x0 0x00010000>,
+              <0x0  0x82900000 0x0 0x00001000>;
+        reg-names = "dbi", "dbi2", "atu", "config", "link";
+        device_type = "pci";
+        #address-cells = <3>;
+        #size-cells = <2>;
+        clocks = <&syscon_apmu 89>,
+                 <&syscon_apmu 56>,
+                 <&syscon_apmu 57>;
+        clock-names = "dbi", "mstr", "slv";
+        interrupts = <141 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "app";
+        msi-parent = <&simsic>;
+        ranges = <0x01000000 0x00 0x00010000 0x11 0x00010000 0x0 0x00100000>,
+                 <0x02000000 0x0  0x00110000 0x11 0x00110000 0x0 0x7fef0000>,
+                 <0x43000000 0x18 0x00000000 0x18 0x00000000 0x1 0x00000000>;
+        resets = <&syscon_apmu 76>,
+                 <&syscon_apmu 78>,
+                 <&syscon_apmu 77>;
+        reset-names = "dbi", "mstr", "slv";
+        linux,pci-domain = <0>;
+        spacemit,apmu = <&syscon_apmu 0x1f0>;
+      };
+    };
+
-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Inochi Amaoto]

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 phy 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/pcie-spacemit-k1.c | 238 ++++++++++++++++++
 1 file changed, 238 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-spacemit-k1.c b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
index f2a722e5edb5..fa529ac18f2d 100644
--- a/drivers/pci/controller/dwc/pcie-spacemit-k1.c
+++ b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
@@ -23,6 +23,7 @@
 
 #define PCI_VENDOR_ID_SPACEMIT		0x201f
 #define PCI_DEVICE_ID_SPACEMIT_K1	0x0001
+#define PCI_DEVICE_ID_SPACEMIT_K3	0x0002
 
 /* Offsets and field definitions for link management registers */
 #define K1_PHY_AHB_IRQ_EN			0x0000
@@ -32,8 +33,27 @@
 #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_PHY_AHB_IRQENABLE_SET_INTX		0x000c
+#define LEG_EP_INTERRUPTS		(BIT(6) | BIT(7) | BIT(8) | BIT(9))
+
+#define K3_PHY_AHB_IRQENABLE_SET_MSI		0x0014
+/* MSI defined as BIT(11) in existing INTR_ENABLE, reusing */
+
+#define K3_ADDR_INTR_STATUS1			0x0018
+
+#define K3_ADDR_INTR_ENABLE1			0x001C
+#define MSI_INT				BIT(0)
+#define MSIX_INT			GENMASK(8, 1)
+
+#define K3_MAX_PHY_NUMBER		6
 
 /* Some controls require APMU regmap access */
 #define SYSCON_APMU			"spacemit,apmu"
@@ -48,6 +68,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 {
 	struct dw_pcie pci;
@@ -263,6 +286,213 @@ static const struct dw_pcie_ops k1_pcie_ops = {
 	.stop_link	= k1_pcie_stop_link,
 };
 
+static int k3_pcie_enable_phy(struct k1_pcie *pcie)
+{
+	int i, ret;
+
+	for (i = 0; i < pcie->phy_count; i++) {
+		ret = phy_init(pcie->phy[i]);
+		if (ret)
+			goto err_phy;
+	}
+
+	return 0;
+
+err_phy:
+	while (--i >= 0)
+		phy_exit(pcie->phy[i]);
+
+	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);
+
+	ret = k1_pcie_enable_resources(k1);
+	if (ret)
+		return ret;
+
+	regmap_set_bits(k1->pmu, reset_ctrl, PCIE_AUX_PWR_DET);
+	regmap_clear_bits(k1->pmu, reset_ctrl, APP_HOLD_PHY_RST);
+
+	ret = k3_pcie_enable_phy(k1);
+	if (ret)
+		return ret;
+
+	/* K3: Set IGNORE_PERSTN and drive PERSTN_OE high (assert reset) */
+	regmap_set_bits(k1->pmu, k1->pmu_off + PCIE_CONTROL_LOGIC,
+			PCIE_IGNORE_PERSTN | PCIE_PERSTN_OE | PCIE_PERSTN_OUT);
+	usleep_range(1000, 2000);
+	regmap_clear_bits(k1->pmu, k1->pmu_off + PCIE_CONTROL_LOGIC, PCIE_PERSTN_OUT);
+
+	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, k1->pmu_off + PCIE_CONTROL_LOGIC,
+			PCIE_PERSTN_OUT | PCIE_PERSTN_OE);
+
+	val = dw_pcie_readl_dbi(pci, GEN3_EQ_CONTROL_OFF);
+	val &= ~(0xffff << 8);
+	val |= ((0x1 << 4) << 8);
+	dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, val);
+
+	/* 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_K3);
+	dw_pcie_dbi_ro_wr_dis(pci);
+
+	/* Finally, as a workaround, disable ASPM L1 */
+	k1_pcie_disable_aspm_l1(k1);
+
+	return 0;
+}
+
+static int k3_pcie_msi_host_init(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	u32 val;
+
+	dw_pcie_dbi_ro_wr_en(pci);
+
+	val = dw_pcie_readl_dbi(pci, COHERENCY_CONTROL_3_OFF);
+	val |= (0xf << 11);
+	dw_pcie_writel_dbi(pci, COHERENCY_CONTROL_3_OFF, val);
+
+	dw_pcie_dbi_ro_wr_dis(pci);
+
+	return 0;
+}
+
+static const struct dw_pcie_host_ops k3_pcie_host_ops = {
+	.init		= k3_pcie_init,
+	.deinit		= k1_pcie_deinit,
+	.msi_init	= k3_pcie_msi_host_init,
+};
+
+static int k3_pcie_start_link(struct dw_pcie *pci)
+{
+	struct k1_pcie *k1 = to_k1_pcie(pci);
+	u32 val;
+
+	k1_pcie_start_link(pci);
+
+	/* Enable INTx */
+	val = readl_relaxed(k1->link + K3_PHY_AHB_IRQENABLE_SET_INTX);
+	val |= LEG_EP_INTERRUPTS;
+	writel_relaxed(val, k1->link + K3_PHY_AHB_IRQENABLE_SET_INTX);
+
+	/* Enable MSI/MSIX specific to K3 */
+	val = readl_relaxed(k1->link + K3_ADDR_INTR_ENABLE1);
+	val |= (MSI_INT | MSIX_INT);
+	writel_relaxed(val, k1->link + K3_ADDR_INTR_ENABLE1);
+
+	return 0;
+}
+
+static const struct dw_pcie_ops k3_pcie_ops = {
+	.link_up	= k1_pcie_link_up,
+	.start_link	= k3_pcie_start_link,
+	.stop_link	= k1_pcie_stop_link,
+};
+
+static void k3_pcie_clear_irq_status(struct k1_pcie *k1,
+				     u32 *status0, u32 *status1, u32 *status2)
+{
+	*status0 = readl_relaxed(k1->link + K3_PHY_AHB_IRQSTATUS_INTX);
+	*status1 = readl_relaxed(k1->link + INTR_STATUS);
+	*status2 = readl_relaxed(k1->link + K3_ADDR_INTR_STATUS1);
+
+	writel_relaxed(*status0, k1->link + K3_PHY_AHB_IRQSTATUS_INTX);
+	writel_relaxed(*status1, k1->link + INTR_STATUS);
+	writel_relaxed(*status2, k1->link + K3_ADDR_INTR_STATUS1);
+}
+
+static int k3_pcie_parse_port(struct k1_pcie *k1)
+{
+	struct device *dev = k1->pci.dev;
+	u32 status0, status1, status2;
+	int i;
+
+	k1->phy = devm_kmalloc_array(dev, sizeof(*k1->phy),
+				     K3_MAX_PHY_NUMBER, GFP_KERNEL);
+	if (IS_ERR(k1->phy))
+		return PTR_ERR(k1->phy);
+
+	for (i = 0; i < K3_MAX_PHY_NUMBER; i++) {
+		k1->phy[i] = devm_of_phy_get_by_index(dev, dev->of_node, i);
+		if (IS_ERR(k1->phy[i])) {
+			if (PTR_ERR(k1->phy[i]) == -ENODEV)
+				break;
+
+			return PTR_ERR(k1->phy[i]);
+		}
+	}
+
+	k1->phy_count = i;
+	if (k1->phy_count == 0)
+		return -EINVAL;
+
+	k3_pcie_clear_irq_status(k1, &status0, &status1, &status2);
+
+	return 0;
+}
+
+static irqreturn_t k3_pcie_irq_thread(int irq, void *data)
+{
+	struct k1_pcie *k1 = data;
+	struct dw_pcie_rp *pp = &k1->pci.pp;
+	struct device *dev = k1->pci.dev;
+	u32 status0, status1, status2;
+
+	k3_pcie_clear_irq_status(k1, &status0, &status1, &status2);
+
+	writel_relaxed(status0, k1->link + K3_PHY_AHB_IRQSTATUS_INTX);
+	writel_relaxed(status1, k1->link + INTR_STATUS);
+	writel_relaxed(status2, k1->link + K3_ADDR_INTR_STATUS1);
+
+	if (FIELD_GET(RDLH_LINK_UP_INT, status1)) {
+		msleep(PCIE_RESET_CONFIG_WAIT_MS);
+		/* Rescan the bus to enumerate endpoint devices */
+		pci_lock_rescan_remove();
+		pci_rescan_bus(pp->bridge->bus);
+		pci_unlock_rescan_remove();
+	} else if (!status0 && !status1 && !status2)
+		dev_WARN_ONCE(dev, true,
+			      "Received unknown event. status0=0x%08x status1=0x%08x status2=0x%08x\n",
+			      status0, status1, status2);
+
+	return IRQ_HANDLED;
+}
+
+static int k3_pcie_post_init(struct k1_pcie *k1)
+{
+	struct device *dev = k1->pci.dev;
+	struct platform_device *pdev = to_platform_device(dev);
+	int irq;
+
+	irq = platform_get_irq_byname_optional(pdev, "app");
+	if (irq > 0) {
+		return devm_request_threaded_irq(dev, irq, NULL,
+						 k3_pcie_irq_thread,
+						 IRQF_ONESHOT, NULL, k1);
+	}
+
+	return 0;
+}
+
 static int k1_pcie_parse_port(struct k1_pcie *k1)
 {
 	struct device *dev = k1->pci.dev;
@@ -373,8 +603,16 @@ static const struct k1_pcie_device_data k1_pcie_device_data = {
 	.parse_port	= k1_pcie_parse_port,
 };
 
+static const struct k1_pcie_device_data k3_pcie_device_data = {
+	.host_ops	= &k3_pcie_host_ops,
+	.ops		= &k3_pcie_ops,
+	.parse_port	= k3_pcie_parse_port,
+	.post_init	= k3_pcie_post_init,
+};
+
 static const struct of_device_id k1_pcie_of_match_table[] = {
 	{ .compatible = "spacemit,k1-pcie", .data = &k1_pcie_device_data},
+	{ .compatible = "spacemit,k3-pcie", .data = &k3_pcie_device_data},
 	{ }
 };
 
-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 3/5] dt-bindings: PCI: snps,dw-pcie: Add msi-parent for msi handle check

[Rob Herring (Arm)]

On Sat, 02 May 2026 18:13:16 +0800, Inochi Amaoto wrote:
> The IMSIC device on RISC-V based system does not require ID
> remapping for MSI. So this device only needs "msi-parent"
> property for IMSIC-based SoC, and the "msi-map" is not a
> necessary property.
> 
> Add new condition for msi handling on IMSIC based SoC.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 4/5] dt-bindings: pci: spacemit: Introduce Spacemit K3 PCIe host controller

[Rob Herring]

On Sat, May 02, 2026 at 06:13:17PM +0800, Inochi Amaoto wrote:
> Add binding support for the PCIe controller on the SpacemiT K3 SoC.
> This controller is almost a standard Synopsys Designware PCIe IP,
> with some extra link and reset state control.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  .../bindings/pci/spacemit,k3-pcie-host.yaml   | 142 ++++++++++++++++++
>  1 file changed, 142 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml b/Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml
> new file mode 100644
> index 000000000000..be2641526b19
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml
> @@ -0,0 +1,142 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/spacemit,k3-pcie-host.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SpacemiT K3 PCI Express Host Controller
> +
> +maintainers:
> +  - Inochi Amaoto <inochiama@gmail.com>
> +
> +description:
> +  The SpacemiT K3 SoC PCIe host controller is based on the Synopsys
> +  DesignWare PCIe IP.  The controller uses the external MSI interrupt
> +  controller.
> +
> +allOf:
> +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> +
> +properties:
> +  compatible:
> +    const: spacemit,k3-pcie
> +
> +  reg:
> +    items:
> +      - description: DesignWare PCIe registers
> +      - description: Data Bus Interface (DBI) shadow registers
> +      - description: ATU address space
> +      - description: PCIe configuration space
> +      - description: Link control registers
> +
> +  reg-names:
> +    items:
> +      - const: dbi
> +      - const: dbi2
> +      - const: atu
> +      - const: config
> +      - const: link
> +
> +  clocks:
> +    items:
> +      - description: DWC PCIe Data Bus Interface (DBI) clock
> +      - description: DWC PCIe application AXI-bus master interface clock
> +      - description: DWC PCIe application AXI-bus slave interface clock
> +
> +  clock-names:
> +    items:
> +      - const: dbi
> +      - const: mstr
> +      - const: slv
> +
> +  resets:
> +    items:
> +      - description: DWC PCIe Data Bus Interface (DBI) reset
> +      - description: DWC PCIe application AXI-bus master interface reset
> +      - description: DWC PCIe application AXI-bus slave interface reset
> +
> +  reset-names:
> +    items:
> +      - const: dbi
> +      - const: mstr
> +      - const: slv
> +
> +  interrupts:
> +    items:
> +      - description: Interrupt used for port state
> +
> +  interrupt-names:
> +    const: app
> +
> +  msi-parent: true
> +
> +  phys:
> +    minItems: 1
> +    maxItems: 6

You have to define what each entry is. I assume this is 1 per lane 
though I thought only a power of 2 number of lanes was valid.

Rob

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Bjorn Helgaas]

On Sat, May 02, 2026 at 06:13:18PM +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 phy at the same time.
> 
> Add driver to support PCIe controller on Spacemit K3 PCIe.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>

Sashiko had some good questions:
https://sashiko.dev/#/patchset/20260502101319.2364052-1-inochiama%40gmail.com

Looks like the CONFIG_PCIE_SPACEMIT_K1 menu item and help text
drivers/pci/controller/dwc/Kconfig should be updated to include K3.

The "CONFIG_PCIE_SPACEMIT_K1" name itself should stay the same.

s/Designware/DesignWare/, also in 4/5 commit log
s/phy/PHY/ here and other patches and subject lines
s/msi/MSI/ in 3/5 subject and commit log when it's a stand-alone word
s/pci:/PCI:/ in 4/5 subject to match history (and patch 3/5)

> +++ b/drivers/pci/controller/dwc/pcie-spacemit-k1.c

> +#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_PHY_AHB_IRQENABLE_SET_INTX		0x000c
> +#define LEG_EP_INTERRUPTS		(BIT(6) | BIT(7) | BIT(8) | BIT(9))

Would be nicer to use "INTX" rather than "LEG" here since we use
"INTX" in K3_PHY_AHB_IRQENABLE_SET_INTX, in the comments, etc.

> +#define K3_PHY_AHB_IRQENABLE_SET_MSI		0x0014
> +/* MSI defined as BIT(11) in existing INTR_ENABLE, reusing */
> +
> +#define K3_ADDR_INTR_STATUS1			0x0018
> +
> +#define K3_ADDR_INTR_ENABLE1			0x001C

You're using a mix of upper- and lower-case hex here.  Be consistent
and match the existing code.

Seems a little weird to have a mix of "IRQ" names (e.g.,
K1_PHY_AHB_IRQ_EN, K3_PHY_AHB_IRQSTATUS_INTX,
K3_PHY_AHB_IRQENABLE_SET_INTX) and "INTR" names (e.g., INTR_STATUS,
INTR_ENABLE, K3_ADDR_INTR_STATUS1, K3_ADDR_INTR_ENABLE1) when I think
they're really talking about the same concept.

And why do the new K3 names have "ADDR" in the middle when the
existing "INTR_ENABLE" names don't?  It's obvious these are addresses
(well, actually I think they're *offsets*, but no need to be that
detailed).

> +static int k3_pcie_init(struct dw_pcie_rp *pp)
> +{
> ...
> +	val = dw_pcie_readl_dbi(pci, GEN3_EQ_CONTROL_OFF);
> +	val &= ~(0xffff << 8);
> +	val |= ((0x1 << 4) << 8);

Can you use FIELD_MODIFY and some #defines here?

> +	dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, val);
> +
> +	/* Set the PCI vendor and device ID */

Superfluous comment since the code is obvious.

> +	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_K3);
> +	dw_pcie_dbi_ro_wr_dis(pci);
> +
> +	/* Finally, as a workaround, disable ASPM L1 */

I guess this means a device erratum?  It advertises L1 but it doesn't
actually work?

> +	k1_pcie_disable_aspm_l1(k1);

> +static int k3_pcie_msi_host_init(struct dw_pcie_rp *pp)
> +{
> ...
> +	val = dw_pcie_readl_dbi(pci, COHERENCY_CONTROL_3_OFF);
> +	val |= (0xf << 11);

FIELD_MODIFY and some #defines here?

> +static int k3_pcie_start_link(struct dw_pcie *pci)
> +{
> +	struct k1_pcie *k1 = to_k1_pcie(pci);
> +	u32 val;
> +
> +	k1_pcie_start_link(pci);
> +
> +	/* Enable INTx */
> +	val = readl_relaxed(k1->link + K3_PHY_AHB_IRQENABLE_SET_INTX);
> +	val |= LEG_EP_INTERRUPTS;
> +	writel_relaxed(val, k1->link + K3_PHY_AHB_IRQENABLE_SET_INTX);
> +
> +	/* Enable MSI/MSIX specific to K3 */

s/MSIX/MSI-X/ to match spec usage.

> +	val = readl_relaxed(k1->link + K3_ADDR_INTR_ENABLE1);
> +	val |= (MSI_INT | MSIX_INT);
> +	writel_relaxed(val, k1->link + K3_ADDR_INTR_ENABLE1);

Generally speaking I think the interrupt setup belongs somewhere other
than .start_link().  Usually .start_link() only enables LTSSM.

> +	return 0;
> +}

> +static irqreturn_t k3_pcie_irq_thread(int irq, void *data)
> +{
> +	struct k1_pcie *k1 = data;
> +	struct dw_pcie_rp *pp = &k1->pci.pp;
> +	struct device *dev = k1->pci.dev;
> +	u32 status0, status1, status2;
> +
> +	k3_pcie_clear_irq_status(k1, &status0, &status1, &status2);
> +
> +	writel_relaxed(status0, k1->link + K3_PHY_AHB_IRQSTATUS_INTX);
> +	writel_relaxed(status1, k1->link + INTR_STATUS);
> +	writel_relaxed(status2, k1->link + K3_ADDR_INTR_STATUS1);
> +
> +	if (FIELD_GET(RDLH_LINK_UP_INT, status1)) {
> +		msleep(PCIE_RESET_CONFIG_WAIT_MS);
> +		/* Rescan the bus to enumerate endpoint devices */
> +		pci_lock_rescan_remove();
> +		pci_rescan_bus(pp->bridge->bus);

This is the *only* driver that uses pci_rescan_bus() this way, which
automatically makes it suspicous.  Maybe it's the first hardware that
implements or is willing to use RDLH_LINK_UP_INT for this, but somehow
I doubt it.

> +		pci_unlock_rescan_remove();
> +	} else if (!status0 && !status1 && !status2)
> +		dev_WARN_ONCE(dev, true,
> +			      "Received unknown event. status0=0x%08x status1=0x%08x status2=0x%08x\n",
> +			      status0, status1, status2);
> +
> +	return IRQ_HANDLED;
> +}

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 4/5] dt-bindings: pci: spacemit: Introduce Spacemit K3 PCIe host controller

[Inochi Amaoto]

On Thu, May 07, 2026 at 02:13:02PM -0500, Rob Herring wrote:
> On Sat, May 02, 2026 at 06:13:17PM +0800, Inochi Amaoto wrote:
> > Add binding support for the PCIe controller on the SpacemiT K3 SoC.
> > This controller is almost a standard Synopsys Designware PCIe IP,
> > with some extra link and reset state control.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> >  .../bindings/pci/spacemit,k3-pcie-host.yaml   | 142 ++++++++++++++++++
> >  1 file changed, 142 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml b/Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml
> > new file mode 100644
> > index 000000000000..be2641526b19
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml
> > @@ -0,0 +1,142 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/spacemit,k3-pcie-host.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: SpacemiT K3 PCI Express Host Controller
> > +
> > +maintainers:
> > +  - Inochi Amaoto <inochiama@gmail.com>
> > +
> > +description:
> > +  The SpacemiT K3 SoC PCIe host controller is based on the Synopsys
> > +  DesignWare PCIe IP.  The controller uses the external MSI interrupt
> > +  controller.
> > +
> > +allOf:
> > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: spacemit,k3-pcie
> > +
> > +  reg:
> > +    items:
> > +      - description: DesignWare PCIe registers
> > +      - description: Data Bus Interface (DBI) shadow registers
> > +      - description: ATU address space
> > +      - description: PCIe configuration space
> > +      - description: Link control registers
> > +
> > +  reg-names:
> > +    items:
> > +      - const: dbi
> > +      - const: dbi2
> > +      - const: atu
> > +      - const: config
> > +      - const: link
> > +
> > +  clocks:
> > +    items:
> > +      - description: DWC PCIe Data Bus Interface (DBI) clock
> > +      - description: DWC PCIe application AXI-bus master interface clock
> > +      - description: DWC PCIe application AXI-bus slave interface clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: dbi
> > +      - const: mstr
> > +      - const: slv
> > +
> > +  resets:
> > +    items:
> > +      - description: DWC PCIe Data Bus Interface (DBI) reset
> > +      - description: DWC PCIe application AXI-bus master interface reset
> > +      - description: DWC PCIe application AXI-bus slave interface reset
> > +
> > +  reset-names:
> > +    items:
> > +      - const: dbi
> > +      - const: mstr
> > +      - const: slv
> > +
> > +  interrupts:
> > +    items:
> > +      - description: Interrupt used for port state
> > +
> > +  interrupt-names:
> > +    const: app
> > +
> > +  msi-parent: true
> > +
> > +  phys:
> > +    minItems: 1
> > +    maxItems: 6
> 
> You have to define what each entry is. I assume this is 1 per lane 
> though I thought only a power of 2 number of lanes was valid.
> 

In fact it is not 1 per lane, the PCIe accept lanes from the Comb PHY,
and the phy can provide 1 lane or 2 lanes according to the PHY MUX. 
In detail,
- PHY 0,1 is 2 lanes
- PHY 2,3,4,5 is 1 lane.

So the max number of the phys is 6 with 8 lanes.

Maybe need a description link to the phy mux configuration?

Regards,
Inochi



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Inochi Amaoto]

On Thu, May 07, 2026 at 05:42:17PM -0500, Bjorn Helgaas wrote:
> On Sat, May 02, 2026 at 06:13:18PM +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 phy at the same time.
> > 
> > Add driver to support PCIe controller on Spacemit K3 PCIe.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> 
> Sashiko had some good questions:
> https://sashiko.dev/#/patchset/20260502101319.2364052-1-inochiama%40gmail.com
> 
> Looks like the CONFIG_PCIE_SPACEMIT_K1 menu item and help text
> drivers/pci/controller/dwc/Kconfig should be updated to include K3.
> 
> The "CONFIG_PCIE_SPACEMIT_K1" name itself should stay the same.
> 
> s/Designware/DesignWare/, also in 4/5 commit log
> s/phy/PHY/ here and other patches and subject lines
> s/msi/MSI/ in 3/5 subject and commit log when it's a stand-alone word
> s/pci:/PCI:/ in 4/5 subject to match history (and patch 3/5)
> 

See it, thanks, I will take care of that

> > +++ b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> 
> > +#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_PHY_AHB_IRQENABLE_SET_INTX		0x000c
> > +#define LEG_EP_INTERRUPTS		(BIT(6) | BIT(7) | BIT(8) | BIT(9))
> 
> Would be nicer to use "INTX" rather than "LEG" here since we use
> "INTX" in K3_PHY_AHB_IRQENABLE_SET_INTX, in the comments, etc.
> 
> > +#define K3_PHY_AHB_IRQENABLE_SET_MSI		0x0014
> > +/* MSI defined as BIT(11) in existing INTR_ENABLE, reusing */
> > +
> > +#define K3_ADDR_INTR_STATUS1			0x0018
> > +
> > +#define K3_ADDR_INTR_ENABLE1			0x001C
> 
> You're using a mix of upper- and lower-case hex here.  Be consistent
> and match the existing code.
> 
> Seems a little weird to have a mix of "IRQ" names (e.g.,
> K1_PHY_AHB_IRQ_EN, K3_PHY_AHB_IRQSTATUS_INTX,
> K3_PHY_AHB_IRQENABLE_SET_INTX) and "INTR" names (e.g., INTR_STATUS,
> INTR_ENABLE, K3_ADDR_INTR_STATUS1, K3_ADDR_INTR_ENABLE1) when I think
> they're really talking about the same concept.
> 
> And why do the new K3 names have "ADDR" in the middle when the
> existing "INTR_ENABLE" names don't?  It's obvious these are addresses
> (well, actually I think they're *offsets*, but no need to be that
> detailed).
> 

In fact I have no detailed document about these name, but reference
to their comments, I think it is a register for some link features.
So it could be more accurate to be named with "LINK"

> > +static int k3_pcie_init(struct dw_pcie_rp *pp)
> > +{
> > ...
> > +	val = dw_pcie_readl_dbi(pci, GEN3_EQ_CONTROL_OFF);
> > +	val &= ~(0xffff << 8);
> > +	val |= ((0x1 << 4) << 8);
> 
> Can you use FIELD_MODIFY and some #defines here?

It is fine for me.

> 
> > +	dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, val);
> > +
> > +	/* Set the PCI vendor and device ID */
> 
> Superfluous comment since the code is obvious.
> 

OK, I will remove it

> > +	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_K3);
> > +	dw_pcie_dbi_ro_wr_dis(pci);
> > +
> > +	/* Finally, as a workaround, disable ASPM L1 */
> 
> I guess this means a device erratum?  It advertises L1 but it doesn't
> actually work?
> 
> > +	k1_pcie_disable_aspm_l1(k1);
> 
> > +static int k3_pcie_msi_host_init(struct dw_pcie_rp *pp)
> > +{
> > ...
> > +	val = dw_pcie_readl_dbi(pci, COHERENCY_CONTROL_3_OFF);
> > +	val |= (0xf << 11);
> 
> FIELD_MODIFY and some #defines here?
> 

OK.

> > +static int k3_pcie_start_link(struct dw_pcie *pci)
> > +{
> > +	struct k1_pcie *k1 = to_k1_pcie(pci);
> > +	u32 val;
> > +
> > +	k1_pcie_start_link(pci);
> > +
> > +	/* Enable INTx */
> > +	val = readl_relaxed(k1->link + K3_PHY_AHB_IRQENABLE_SET_INTX);
> > +	val |= LEG_EP_INTERRUPTS;
> > +	writel_relaxed(val, k1->link + K3_PHY_AHB_IRQENABLE_SET_INTX);
> > +
> > +	/* Enable MSI/MSIX specific to K3 */
> 
> s/MSIX/MSI-X/ to match spec usage.
> 
> > +	val = readl_relaxed(k1->link + K3_ADDR_INTR_ENABLE1);
> > +	val |= (MSI_INT | MSIX_INT);
> > +	writel_relaxed(val, k1->link + K3_ADDR_INTR_ENABLE1);
> 
> Generally speaking I think the interrupt setup belongs somewhere other
> than .start_link().  Usually .start_link() only enables LTSSM.
> 

Yes, this logic are not needed any more after I recheck the vendor
code. Only thing related to the link will be left.

With this, the macro like LEG_EP_INTERRUPTS can be removed.

> > +	return 0;
> > +}
> 
> > +static irqreturn_t k3_pcie_irq_thread(int irq, void *data)
> > +{
> > +	struct k1_pcie *k1 = data;
> > +	struct dw_pcie_rp *pp = &k1->pci.pp;
> > +	struct device *dev = k1->pci.dev;
> > +	u32 status0, status1, status2;
> > +
> > +	k3_pcie_clear_irq_status(k1, &status0, &status1, &status2);
> > +
> > +	writel_relaxed(status0, k1->link + K3_PHY_AHB_IRQSTATUS_INTX);
> > +	writel_relaxed(status1, k1->link + INTR_STATUS);
> > +	writel_relaxed(status2, k1->link + K3_ADDR_INTR_STATUS1);
> > +
> > +	if (FIELD_GET(RDLH_LINK_UP_INT, status1)) {
> > +		msleep(PCIE_RESET_CONFIG_WAIT_MS);
> > +		/* Rescan the bus to enumerate endpoint devices */
> > +		pci_lock_rescan_remove();
> > +		pci_rescan_bus(pp->bridge->bus);
> 
> This is the *only* driver that uses pci_rescan_bus() this way, which
> automatically makes it suspicous.  Maybe it's the first hardware that
> implements or is willing to use RDLH_LINK_UP_INT for this, but somehow
> I doubt it.
>

I am going to remove this. At least I do not think it is very proper
to add this in the first version. 
The vendor explained that they use this interrupt to speed up the
device link up check. This depends on a feature that make dwc skip
the link up delay. And this feature is removed in v7.0. In commit
142d5869f6ee ("Revert "PCI: dwc: Don't wait for link up if driver
can detect Link Up event"")

Regards,
Inochi
 
> > +		pci_unlock_rescan_remove();
> > +	} else if (!status0 && !status1 && !status2)
> > +		dev_WARN_ONCE(dev, true,
> > +			      "Received unknown event. status0=0x%08x status1=0x%08x status2=0x%08x\n",
> > +			      status0, status1, status2);
> > +
> > +	return IRQ_HANDLED;
> > +}

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv