[PATCH v11 0/7] Add PCIe support for Qualcomm IPQ5332

[PATCH v11 0/7] Add PCIe support for Qualcomm IPQ5332

[Varadarajan Narayanan]

Patch series adds support for enabling the PCIe controller and
UNIPHY found on Qualcomm IPQ5332 platform. PCIe0 is Gen3 X1 and
PCIe1 is Gen3 X2 are added.

This series combines [1] and [2]. [1] introduces IPQ5018 PCIe
support and [2] depends on [1] to introduce IPQ5332 PCIe support.
Since the community was interested in [2] (please see [3]), tried
to revive IPQ5332's PCIe support with v2 of this patch series.

v2 of this series pulled in the phy driver from [1] tried to
address comments/feedback given in both [1] and [2].

1. Enable IPQ5018 PCI support (Nitheesh Sekar) - https://lore.kernel.org/all/20231003120846.28626-1-quic_nsekar@quicinc.com/
2. Add PCIe support for Qualcomm IPQ5332 (Praveenkumar I) - https://lore.kernel.org/linux-arm-msm/20231214062847.2215542-1-quic_ipkumar@quicinc.com/
3. Community interest - https://lore.kernel.org/linux-arm-msm/20240310132915.GE3390@thinkpad/

v11: * phy-qcom-uniphy-pcie-28lp.c
	 * Remove unused #define
	 * Use "250 * MEGA" instead of 250000000

v10: * ipq5332.dtsi: Trim down the list of assigned clocks

     * ipq9574 and ipq5332 DT
	 * Fix 'simple-bus unit address format error' in ipq9574 and
	   ipq5332 DTS
         * Rearrange nodes w.r.t. address sort order

     * Have spoken with 'Manikanta Mylavarapu' [1] for omitting similar
       changes in qcom,pcie.yaml that are handled in this series.

     * Reformat commit messages to 75 character limit

     * controller bindings:
       Fix maxItems for interrupts constraint of sdm845

     1 - https://lore.kernel.org/linux-arm-msm/20250125035920.2651972-2-quic_mmanikan@quicinc.com/

v9: Dont have fallback for num-lanes in driver and return error
    Remove superfluous ipq5332 constraint as the fallback is present

v8: Add reviewed by
    Remove duplication in bindings due to ipq5424 code getting merged

v7: phy bindings:
    * Include data type definition to 'num-lanes'

    controller bindings:
    * Split the ipq9574 and ipq5332 changes into separate patches

    dtsi:
    * Add root port definitions

v6: phy bindings:
    * Fix num-lanes definition

    phy driver:
    * Fix num-lanes handling in probe to use generally followed pattern

    controller bindings:
    * Give more info in commit log

    dtsi:
    * Add assigned-clocks & assigned-clock-rates to controller nodes
    * Add num-lanes to pcie0_phy

v5: phy bindings:
    * Drop '3x1' & '3x2' from compatible string
    * Use 'num-lanes' to differentiate instead of '3x1' or '3x2'
      in compatible string
    * Describe clocks and resets instead of just maxItems

    phy driver:
    * Get num-lanes from DTS
    * Drop compatible specific init data as there is only one
      compatible string

    controller bindings:
    * Re-arrange 5332 and 9574 compatibles to handle fallback usage in dts

    dtsi:
    * Add 'num-lanes' to "pcie1_phy: phy@4b1000"
    * Make ipq5332 as main and ipq9574 as fallback compatible
    * Sort controller nodes per address

    misc:
    Add R-B tag from Konrad to dts and dtsi patches

v4: * phy bindings - Create ipq5332 compatible instead of reusing ipq9574 for bindings
    * phy bindings - Remove reset-names as the resets are handled with bulk APIs
    * phy bindings - Fix order in the 'required' section
    * phy bindings - Remove clock-output-names
    * dtsi - Add missing reset for pcie1_phy
    * dtsi - Convert 'reg-names' to a vertical list
    * dts - Fix nodes sort order
    * dts - Use property-n followed by property-names

v3: * Update the cover letter with the sources of the patches
    * Rename the dt-bindings yaml file similar to other phys
    * Drop ipq5332 specific pcie controllor bindings and reuse
      ipq9574 pcie controller bindings for ipq5332
    * Please see patches for specific changes
    * Set GPL license for phy-qcom-uniphy-pcie-28lp.c

v2: Address review comments from V1
    Drop the 'required clocks' change that would break ABI (in dt-binding, dts, gcc-ipq5332.c)
    Include phy driver from the dependent series

v1: https://lore.kernel.org/linux-arm-msm/20231214062847.2215542-1-quic_ipkumar@quicinc.com/

Nitheesh Sekar (2):
  dt-bindings: phy: qcom,uniphy-pcie: Document PCIe uniphy
  phy: qcom: Introduce PCIe UNIPHY 28LP driver

Praveenkumar I (2):
  arm64: dts: qcom: ipq5332: Add PCIe related nodes
  arm64: dts: qcom: ipq5332-rdp441: Enable PCIe phys and controllers

Varadarajan Narayanan (3):
  dt-bindings: PCI: qcom: Use sdx55 reg description for ipq9574
  arm64: dts: qcom: ipq9574: Reorder reg and reg-names
  dt-bindings: PCI: qcom: Document the IPQ5332 PCIe controller

 .../devicetree/bindings/pci/qcom,pcie.yaml    |  10 +-
 .../phy/qcom,ipq5332-uniphy-pcie-phy.yaml     |  76 ++
 arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts   |  76 ++
 arch/arm64/boot/dts/qcom/ipq5332.dtsi         | 252 +++++-
 arch/arm64/boot/dts/qcom/ipq9574.dtsi         | 837 +++++++++---------
 drivers/phy/qualcomm/Kconfig                  |  12 +
 drivers/phy/qualcomm/Makefile                 |   1 +
 .../phy/qualcomm/phy-qcom-uniphy-pcie-28lp.c  | 286 ++++++
 8 files changed, 1134 insertions(+), 416 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq5332-uniphy-pcie-phy.yaml
 create mode 100644 drivers/phy/qualcomm/phy-qcom-uniphy-pcie-28lp.c


base-commit: e5d3fd687aac5eceb1721fa92b9f49afcf4c3717
-- 
2.34.1

[PATCH v11 1/7] dt-bindings: phy: qcom,uniphy-pcie: Document PCIe uniphy

[Varadarajan Narayanan]

From: Nitheesh Sekar <quic_nsekar@quicinc.com>

Document the Qualcomm UNIPHY PCIe 28LP present in IPQ5332.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Nitheesh Sekar <quic_nsekar@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v8: Add 'Reviewed-by: Krzysztof Kozlowski'

v7: * Add data type definition to 'num-lanes'

v6: * Fix num-lanes definition
    * Make it mandatory

v5: * Drop '3x1' & '3x2' from compatible string
    * Use 'num-lanes' to differentiate instead of '3x1' or '3x2'
      in compatible string
    * Describe clocks and resets instead of just maxItems

v4: Remove reset-names as the resets are not used individually
    Remove clock-output-names as its usage is removed from driver
    Fix order in the 'required' section

v3: Fix compatible string to be similar to other phys and rename file accordingly
    Fix clocks minItems -> maxItems
    Change one of the maintainer from Sricharan to Varadarajan

v2: Rename the file to match the compatible
    Drop 'driver' from title
    Dropped 'clock-names'
    Fixed 'reset-names'
---
 .../phy/qcom,ipq5332-uniphy-pcie-phy.yaml     | 76 +++++++++++++++++++
 1 file changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq5332-uniphy-pcie-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-uniphy-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-uniphy-pcie-phy.yaml
new file mode 100644
index 000000000000..e39168d55d23
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,ipq5332-uniphy-pcie-phy.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,ipq5332-uniphy-pcie-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm UNIPHY PCIe 28LP PHY
+
+maintainers:
+  - Nitheesh Sekar <quic_nsekar@quicinc.com>
+  - Varadarajan Narayanan <quic_varada@quicinc.com>
+
+description:
+  PCIe and USB combo PHY found in Qualcomm IPQ5332 SoC
+
+properties:
+  compatible:
+    enum:
+      - qcom,ipq5332-uniphy-pcie-phy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: pcie pipe clock
+      - description: pcie ahb clock
+
+  resets:
+    items:
+      - description: phy reset
+      - description: ahb reset
+      - description: cfg reset
+
+  "#phy-cells":
+    const: 0
+
+  "#clock-cells":
+    const: 0
+
+  num-lanes:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2]
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - resets
+  - "#phy-cells"
+  - "#clock-cells"
+  - num-lanes
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
+
+    pcie0_phy: phy@4b0000 {
+        compatible = "qcom,ipq5332-uniphy-pcie-phy";
+        reg = <0x004b0000 0x800>;
+
+        clocks = <&gcc GCC_PCIE3X1_0_PIPE_CLK>,
+                 <&gcc GCC_PCIE3X1_PHY_AHB_CLK>;
+
+        resets = <&gcc GCC_PCIE3X1_0_PHY_BCR>,
+                 <&gcc GCC_PCIE3X1_PHY_AHB_CLK_ARES>,
+                 <&gcc GCC_PCIE3X1_0_PHY_PHY_BCR>;
+
+        #clock-cells = <0>;
+
+        #phy-cells = <0>;
+
+        num-lanes = <1>;
+    };
-- 
2.34.1

[PATCH v11 2/7] phy: qcom: Introduce PCIe UNIPHY 28LP driver

[Varadarajan Narayanan]

From: Nitheesh Sekar <quic_nsekar@quicinc.com>

Add Qualcomm PCIe UNIPHY 28LP driver support present in Qualcomm IPQ5332
SoC and the phy init sequence.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Nitheesh Sekar <quic_nsekar@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v9: Align with dt-bindings and make num-lanes mandatory. Return error if not
    present
    Add new line before return in one of the functions

v6: Use generally followed pattern for getting num-lanes from DT

v5: * Use 'num-lanes' to differentiate instead of '3x1' or '3x2'
      in compatible string
    * Drop compatible specific init data as there is only one
      compatible string
    * Fix header file order

v4: Fix uppercase hex digit
    Use phy->id for pipe clock source

v3: Added 'Reviewed-by: Dmitry Baryshkov' and made following updates
    s/unsigned int/u32/g
    Fix 'lane_offset' comments
    Fix #define tab -> space
    Fix mixed case hex numbers
    Fix licensing & owner
    Change for-loop pointer to use [] instead of ->
    Use 'less than max' instead of 'not equal to max' in termination condition
    Smatch and Coccinelle passed

v2: Drop IPQ5018 related code and data
    Use uniform prefix for struct names
    Place "}, {", on the same line
    In qcom_uniphy_pcie_init(), use for-loop instead of while
    Swap reset and clock disable order in qcom_uniphy_pcie_power_off
    Add reset assert to qcom_uniphy_pcie_power_on's error path
    Use macros for usleep duration
    Inlined qcom_uniphy_pcie_get_resources & use devm_platform_get_and_ioremap_resource
    Drop 'clock-output-names' from phy_pipe_clk_register
---
 drivers/phy/qualcomm/Kconfig                  |  12 +
 drivers/phy/qualcomm/Makefile                 |   1 +
 .../phy/qualcomm/phy-qcom-uniphy-pcie-28lp.c  | 286 ++++++++++++++++++
 3 files changed, 299 insertions(+)
 create mode 100644 drivers/phy/qualcomm/phy-qcom-uniphy-pcie-28lp.c

diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index 846f8c99547f..a6b71fda1b9c 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -154,6 +154,18 @@ config PHY_QCOM_M31_USB
 	  management. This driver is required even for peripheral only or
 	  host only mode configurations.
 
+config PHY_QCOM_UNIPHY_PCIE_28LP
+	bool "PCIE UNIPHY 28LP PHY driver"
+	depends on ARCH_QCOM
+	depends on HAS_IOMEM
+	depends on OF
+	select GENERIC_PHY
+	help
+	  Enable this to support the PCIe UNIPHY 28LP phy transceiver that
+	  is used with PCIe controllers on Qualcomm IPQ5332 chips. It
+	  handles PHY initialization, clock management required after
+	  resetting the hardware and power management.
+
 config PHY_QCOM_USB_HS
 	tristate "Qualcomm USB HS PHY module"
 	depends on USB_ULPI_BUS
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index eb60e950ad53..42038bc30974 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_PHY_QCOM_QMP_USB_LEGACY)	+= phy-qcom-qmp-usb-legacy.o
 obj-$(CONFIG_PHY_QCOM_QUSB2)		+= phy-qcom-qusb2.o
 obj-$(CONFIG_PHY_QCOM_SNPS_EUSB2)	+= phy-qcom-snps-eusb2.o
 obj-$(CONFIG_PHY_QCOM_EUSB2_REPEATER)	+= phy-qcom-eusb2-repeater.o
+obj-$(CONFIG_PHY_QCOM_UNIPHY_PCIE_28LP)	+= phy-qcom-uniphy-pcie-28lp.o
 obj-$(CONFIG_PHY_QCOM_USB_HS) 		+= phy-qcom-usb-hs.o
 obj-$(CONFIG_PHY_QCOM_USB_HSIC) 	+= phy-qcom-usb-hsic.o
 obj-$(CONFIG_PHY_QCOM_USB_HS_28NM)	+= phy-qcom-usb-hs-28nm.o
diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy-pcie-28lp.c b/drivers/phy/qualcomm/phy-qcom-uniphy-pcie-28lp.c
new file mode 100644
index 000000000000..c8b2a3818880
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-uniphy-pcie-28lp.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2025, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/units.h>
+
+#define RST_ASSERT_DELAY_MIN_US		100
+#define RST_ASSERT_DELAY_MAX_US		150
+#define PIPE_CLK_DELAY_MIN_US		5000
+#define PIPE_CLK_DELAY_MAX_US		5100
+#define CLK_EN_DELAY_MIN_US		30
+#define CLK_EN_DELAY_MAX_US		50
+#define CDR_CTRL_REG_1		0x80
+#define CDR_CTRL_REG_2		0x84
+#define CDR_CTRL_REG_3		0x88
+#define CDR_CTRL_REG_4		0x8c
+#define CDR_CTRL_REG_5		0x90
+#define CDR_CTRL_REG_6		0x94
+#define CDR_CTRL_REG_7		0x98
+#define SSCG_CTRL_REG_1		0x9c
+#define SSCG_CTRL_REG_2		0xa0
+#define SSCG_CTRL_REG_3		0xa4
+#define SSCG_CTRL_REG_4		0xa8
+#define SSCG_CTRL_REG_5		0xac
+#define SSCG_CTRL_REG_6		0xb0
+#define PCS_INTERNAL_CONTROL_2	0x2d8
+
+#define PHY_CFG_PLLCFG				0x220
+#define PHY_CFG_EIOS_DTCT_REG			0x3e4
+#define PHY_CFG_GEN3_ALIGN_HOLDOFF_TIME		0x3e8
+
+enum qcom_uniphy_pcie_type {
+	PHY_TYPE_PCIE = 1,
+	PHY_TYPE_PCIE_GEN2,
+	PHY_TYPE_PCIE_GEN3,
+};
+
+struct qcom_uniphy_pcie_regs {
+	u32 offset;
+	u32 val;
+};
+
+struct qcom_uniphy_pcie_data {
+	int lane_offset; /* offset between the lane register bases */
+	u32 phy_type;
+	const struct qcom_uniphy_pcie_regs *init_seq;
+	u32 init_seq_num;
+	u32 pipe_clk_rate;
+};
+
+struct qcom_uniphy_pcie {
+	struct phy phy;
+	struct device *dev;
+	const struct qcom_uniphy_pcie_data *data;
+	struct clk_bulk_data *clks;
+	int num_clks;
+	struct reset_control *resets;
+	void __iomem *base;
+	int lanes;
+};
+
+#define phy_to_dw_phy(x)	container_of((x), struct qca_uni_pcie_phy, phy)
+
+static const struct qcom_uniphy_pcie_regs ipq5332_regs[] = {
+	{
+		.offset = PHY_CFG_PLLCFG,
+		.val = 0x30,
+	}, {
+		.offset = PHY_CFG_EIOS_DTCT_REG,
+		.val = 0x53ef,
+	}, {
+		.offset = PHY_CFG_GEN3_ALIGN_HOLDOFF_TIME,
+		.val = 0xcf,
+	},
+};
+
+static const struct qcom_uniphy_pcie_data ipq5332_data = {
+	.lane_offset	= 0x800,
+	.phy_type	= PHY_TYPE_PCIE_GEN3,
+	.init_seq	= ipq5332_regs,
+	.init_seq_num	= ARRAY_SIZE(ipq5332_regs),
+	.pipe_clk_rate	= 250 * MEGA,
+};
+
+static void qcom_uniphy_pcie_init(struct qcom_uniphy_pcie *phy)
+{
+	const struct qcom_uniphy_pcie_data *data = phy->data;
+	const struct qcom_uniphy_pcie_regs *init_seq;
+	void __iomem *base = phy->base;
+	int lane, i;
+
+	for (lane = 0; lane < phy->lanes; lane++) {
+		init_seq = data->init_seq;
+
+		for (i = 0; i < data->init_seq_num; i++)
+			writel(init_seq[i].val, base + init_seq[i].offset);
+
+		base += data->lane_offset;
+	}
+}
+
+static int qcom_uniphy_pcie_power_off(struct phy *x)
+{
+	struct qcom_uniphy_pcie *phy = phy_get_drvdata(x);
+
+	clk_bulk_disable_unprepare(phy->num_clks, phy->clks);
+
+	return reset_control_assert(phy->resets);
+}
+
+static int qcom_uniphy_pcie_power_on(struct phy *x)
+{
+	struct qcom_uniphy_pcie *phy = phy_get_drvdata(x);
+	int ret;
+
+	ret = reset_control_assert(phy->resets);
+	if (ret) {
+		dev_err(phy->dev, "reset assert failed (%d)\n", ret);
+		return ret;
+	}
+
+	usleep_range(RST_ASSERT_DELAY_MIN_US, RST_ASSERT_DELAY_MAX_US);
+
+	ret = reset_control_deassert(phy->resets);
+	if (ret) {
+		dev_err(phy->dev, "reset deassert failed (%d)\n", ret);
+		return ret;
+	}
+
+	usleep_range(PIPE_CLK_DELAY_MIN_US, PIPE_CLK_DELAY_MAX_US);
+
+	ret = clk_bulk_prepare_enable(phy->num_clks, phy->clks);
+	if (ret) {
+		dev_err(phy->dev, "clk prepare and enable failed %d\n", ret);
+		return ret;
+	}
+
+	usleep_range(CLK_EN_DELAY_MIN_US, CLK_EN_DELAY_MAX_US);
+
+	qcom_uniphy_pcie_init(phy);
+
+	return 0;
+}
+
+static inline int qcom_uniphy_pcie_get_resources(struct platform_device *pdev,
+						 struct qcom_uniphy_pcie *phy)
+{
+	struct resource *res;
+
+	phy->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	if (IS_ERR(phy->base))
+		return PTR_ERR(phy->base);
+
+	phy->num_clks = devm_clk_bulk_get_all(phy->dev, &phy->clks);
+	if (phy->num_clks < 0)
+		return phy->num_clks;
+
+	phy->resets = devm_reset_control_array_get_exclusive(phy->dev);
+	if (IS_ERR(phy->resets))
+		return PTR_ERR(phy->resets);
+
+	return 0;
+}
+
+/*
+ * Register a fixed rate pipe clock.
+ *
+ * The <s>_pipe_clksrc generated by PHY goes to the GCC that gate
+ * controls it. The <s>_pipe_clk coming out of the GCC is requested
+ * by the PHY driver for its operations.
+ * We register the <s>_pipe_clksrc here. The gcc driver takes care
+ * of assigning this <s>_pipe_clksrc as parent to <s>_pipe_clk.
+ * Below picture shows this relationship.
+ *
+ *         +---------------+
+ *         |   PHY block   |<<---------------------------------------+
+ *         |               |                                         |
+ *         |   +-------+   |                   +-----+               |
+ *   I/P---^-->|  PLL  |---^--->pipe_clksrc--->| GCC |--->pipe_clk---+
+ *    clk  |   +-------+   |                   +-----+
+ *         +---------------+
+ */
+static inline int phy_pipe_clk_register(struct qcom_uniphy_pcie *phy, int id)
+{
+	const struct qcom_uniphy_pcie_data *data = phy->data;
+	struct clk_hw *hw;
+	char name[64];
+
+	snprintf(name, sizeof(name), "phy%d_pipe_clk_src", id);
+	hw = devm_clk_hw_register_fixed_rate(phy->dev, name, NULL, 0,
+					     data->pipe_clk_rate);
+	if (IS_ERR(hw))
+		return dev_err_probe(phy->dev, PTR_ERR(hw),
+				     "Unable to register %s\n", name);
+
+	return devm_of_clk_add_hw_provider(phy->dev, of_clk_hw_simple_get, hw);
+}
+
+static const struct of_device_id qcom_uniphy_pcie_id_table[] = {
+	{
+		.compatible = "qcom,ipq5332-uniphy-pcie-phy",
+		.data = &ipq5332_data,
+	}, {
+		/* Sentinel */
+	},
+};
+MODULE_DEVICE_TABLE(of, qcom_uniphy_pcie_id_table);
+
+static const struct phy_ops pcie_ops = {
+	.power_on	= qcom_uniphy_pcie_power_on,
+	.power_off	= qcom_uniphy_pcie_power_off,
+	.owner          = THIS_MODULE,
+};
+
+static int qcom_uniphy_pcie_probe(struct platform_device *pdev)
+{
+	struct phy_provider *phy_provider;
+	struct device *dev = &pdev->dev;
+	struct qcom_uniphy_pcie *phy;
+	struct phy *generic_phy;
+	int ret;
+
+	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, phy);
+	phy->dev = &pdev->dev;
+
+	phy->data = of_device_get_match_data(dev);
+	if (!phy->data)
+		return -EINVAL;
+
+	ret = of_property_read_u32(dev_of_node(dev), "num-lanes", &phy->lanes);
+	if (ret)
+		return dev_err_probe(dev, ret, "Couldn't read num-lanes\n");
+
+	ret = qcom_uniphy_pcie_get_resources(pdev, phy);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "failed to get resources: %d\n", ret);
+
+	generic_phy = devm_phy_create(phy->dev, NULL, &pcie_ops);
+	if (IS_ERR(generic_phy))
+		return PTR_ERR(generic_phy);
+
+	phy_set_drvdata(generic_phy, phy);
+
+	ret = phy_pipe_clk_register(phy, generic_phy->id);
+	if (ret)
+		dev_err(&pdev->dev, "failed to register phy pipe clk\n");
+
+	phy_provider = devm_of_phy_provider_register(phy->dev,
+						     of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	return 0;
+}
+
+static struct platform_driver qcom_uniphy_pcie_driver = {
+	.probe		= qcom_uniphy_pcie_probe,
+	.driver		= {
+		.name	= "qcom-uniphy-pcie",
+		.of_match_table = qcom_uniphy_pcie_id_table,
+	},
+};
+
+module_platform_driver(qcom_uniphy_pcie_driver);
+
+MODULE_DESCRIPTION("PCIE QCOM UNIPHY driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1

[PATCH v11 3/7] dt-bindings: PCI: qcom: Use sdx55 reg description for ipq9574

[Varadarajan Narayanan]

All DT entries except "reg" is similar between ipq5332 and ipq9574. ipq9574
has 5 registers while ipq5332 has 6. MHI is the additional (i.e. sixth
entry). Since this matches with the sdx55's "reg" definition which allows
for 5 or 6 registers, combine ipq9574 with sdx55.

This change is to prepare ipq9574 to be used as ipq5332's fallback
compatible.

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v8: Add 'Reviewed-by: Krzysztof Kozlowski'
---
 Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 7235d6554cfb..4b4927178abc 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -169,7 +169,6 @@ allOf:
             enum:
               - qcom,pcie-ipq6018
               - qcom,pcie-ipq8074-gen3
-              - qcom,pcie-ipq9574
     then:
       properties:
         reg:
@@ -210,6 +209,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,pcie-ipq9574
               - qcom,pcie-sdx55
     then:
       properties:
-- 
2.34.1

[PATCH v11 4/7] arm64: dts: qcom: ipq9574: Reorder reg and reg-names

[Varadarajan Narayanan]

The 'reg' & 'reg-names' constraints used in the bindings and dtsi are
different resulting in dt_bindings_check errors. Re-order the reg entries,
fix the node names and move the nodes to maintain sort order to address the
following errors/warnings.

	arch/arm64/boot/dts/qcom/ipq9574-rdp449.dtb: pcie@20000000: reg-names:0: 'parf' was expected
	arch/arm64/boot/dts/qcom/ipq9574.dtsi:1045.24-1127.5: Warning (simple_bus_reg): /soc@0/pcie@20000000: simple-bus unit address format error, expected "88000"

Move the nodes to maintain sort order w.r.t address.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v10: Move the nodes to maintain sort order w.r.t address.
     Fix 'simple-bus unit address format error'
---
 arch/arm64/boot/dts/qcom/ipq9574.dtsi | 837 +++++++++++++-------------
 1 file changed, 426 insertions(+), 411 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 942290028972..ab7cb1b5b076 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -239,6 +239,89 @@ rpm_msg_ram: sram@60000 {
 			reg = <0x00060000 0x6000>;
 		};
 
+		pcie0: pci@80000 {
+			compatible = "qcom,pcie-ipq9574";
+			reg =  <0x00080000 0x4000>,
+			       <0x28000000 0xf1d>,
+			       <0x28000f20 0xa8>,
+			       <0x28001000 0x1000>,
+			       <0x28100000 0x1000>;
+			reg-names = "parf",
+				    "dbi",
+				    "elbi",
+				    "atu",
+				    "config";
+			device_type = "pci";
+			linux,pci-domain = <0>;
+			bus-range = <0x00 0xff>;
+			num-lanes = <1>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+
+			ranges = <0x01000000 0x0 0x00000000 0x28200000 0x0 0x100000>,
+				 <0x02000000 0x0 0x28300000 0x28300000 0x0 0x7d00000>;
+			interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi0",
+					  "msi1",
+					  "msi2",
+					  "msi3",
+					  "msi4",
+					  "msi5",
+					  "msi6",
+					  "msi7";
+
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 0x7>;
+			interrupt-map = <0 0 0 1 &intc 0 0 75 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &intc 0 0 78 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &intc 0 0 79 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &intc 0 0 83 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&gcc GCC_PCIE0_AXI_M_CLK>,
+				 <&gcc GCC_PCIE0_AXI_S_CLK>,
+				 <&gcc GCC_PCIE0_AXI_S_BRIDGE_CLK>,
+				 <&gcc GCC_PCIE0_RCHNG_CLK>,
+				 <&gcc GCC_PCIE0_AHB_CLK>,
+				 <&gcc GCC_PCIE0_AUX_CLK>;
+			clock-names = "axi_m",
+				      "axi_s",
+				      "axi_bridge",
+				      "rchng",
+				      "ahb",
+				      "aux";
+
+			resets = <&gcc GCC_PCIE0_PIPE_ARES>,
+				 <&gcc GCC_PCIE0_CORE_STICKY_ARES>,
+				 <&gcc GCC_PCIE0_AXI_S_STICKY_ARES>,
+				 <&gcc GCC_PCIE0_AXI_S_ARES>,
+				 <&gcc GCC_PCIE0_AXI_M_STICKY_ARES>,
+				 <&gcc GCC_PCIE0_AXI_M_ARES>,
+				 <&gcc GCC_PCIE0_AUX_ARES>,
+				 <&gcc GCC_PCIE0_AHB_ARES>;
+			reset-names = "pipe",
+				      "sticky",
+				      "axi_s_sticky",
+				      "axi_s",
+				      "axi_m_sticky",
+				      "axi_m",
+				      "aux",
+				      "ahb";
+
+			phys = <&pcie0_phy>;
+			phy-names = "pciephy";
+			interconnects = <&gcc MASTER_ANOC_PCIE0 &gcc SLAVE_ANOC_PCIE0>,
+					<&gcc MASTER_SNOC_PCIE0 &gcc SLAVE_SNOC_PCIE0>;
+			interconnect-names = "pcie-mem", "cpu-pcie";
+			status = "disabled";
+		};
+
 		pcie0_phy: phy@84000 {
 			compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
 			reg = <0x00084000 0x1000>;
@@ -262,6 +345,90 @@ pcie0_phy: phy@84000 {
 			status = "disabled";
 		};
 
+		pcie2: pcie@88000 {
+			compatible = "qcom,pcie-ipq9574";
+			reg =  <0x00088000 0x4000>,
+			       <0x20000000 0xf1d>,
+			       <0x20000f20 0xa8>,
+			       <0x20001000 0x1000>,
+			       <0x20100000 0x1000>;
+			reg-names = "parf",
+				    "dbi",
+				    "elbi",
+				    "atu",
+				    "config";
+			device_type = "pci";
+			linux,pci-domain = <2>;
+			bus-range = <0x00 0xff>;
+			num-lanes = <2>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+
+			ranges = <0x01000000 0x0 0x00000000 0x20200000 0x0 0x100000>,
+				 <0x02000000 0x0 0x20300000 0x20300000 0x0 0x7d00000>;
+
+			interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 142 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi0",
+					  "msi1",
+					  "msi2",
+					  "msi3",
+					  "msi4",
+					  "msi5",
+					  "msi6",
+					  "msi7";
+
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 0x7>;
+			interrupt-map = <0 0 0 1 &intc 0 0 164 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &intc 0 0 165 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &intc 0 0 186 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &intc 0 0 187 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&gcc GCC_PCIE2_AXI_M_CLK>,
+				 <&gcc GCC_PCIE2_AXI_S_CLK>,
+				 <&gcc GCC_PCIE2_AXI_S_BRIDGE_CLK>,
+				 <&gcc GCC_PCIE2_RCHNG_CLK>,
+				 <&gcc GCC_PCIE2_AHB_CLK>,
+				 <&gcc GCC_PCIE2_AUX_CLK>;
+			clock-names = "axi_m",
+				      "axi_s",
+				      "axi_bridge",
+				      "rchng",
+				      "ahb",
+				      "aux";
+
+			resets = <&gcc GCC_PCIE2_PIPE_ARES>,
+				 <&gcc GCC_PCIE2_CORE_STICKY_ARES>,
+				 <&gcc GCC_PCIE2_AXI_S_STICKY_ARES>,
+				 <&gcc GCC_PCIE2_AXI_S_ARES>,
+				 <&gcc GCC_PCIE2_AXI_M_STICKY_ARES>,
+				 <&gcc GCC_PCIE2_AXI_M_ARES>,
+				 <&gcc GCC_PCIE2_AUX_ARES>,
+				 <&gcc GCC_PCIE2_AHB_ARES>;
+			reset-names = "pipe",
+				      "sticky",
+				      "axi_s_sticky",
+				      "axi_s",
+				      "axi_m_sticky",
+				      "axi_m",
+				      "aux",
+				      "ahb";
+
+			phys = <&pcie2_phy>;
+			phy-names = "pciephy";
+			interconnects = <&gcc MASTER_ANOC_PCIE2 &gcc SLAVE_ANOC_PCIE2>,
+					<&gcc MASTER_SNOC_PCIE2 &gcc SLAVE_SNOC_PCIE2>;
+			interconnect-names = "pcie-mem", "cpu-pcie";
+			status = "disabled";
+		};
+
 		pcie2_phy: phy@8c000 {
 			compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
 			reg = <0x0008c000 0x2000>;
@@ -285,13 +452,6 @@ pcie2_phy: phy@8c000 {
 			status = "disabled";
 		};
 
-		rng: rng@e3000 {
-			compatible = "qcom,ipq9574-trng", "qcom,trng";
-			reg = <0x000e3000 0x1000>;
-			clocks = <&gcc GCC_PRNG_AHB_CLK>;
-			clock-names = "core";
-		};
-
 		mdio: mdio@90000 {
 			compatible = "qcom,ipq9574-mdio", "qcom,ipq4019-mdio";
 			reg = <0x00090000 0x64>;
@@ -302,52 +462,6 @@ mdio: mdio@90000 {
 			status = "disabled";
 		};
 
-		pcie3_phy: phy@f4000 {
-			compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
-			reg = <0x000f4000 0x2000>;
-
-			clocks = <&gcc GCC_PCIE3_AUX_CLK>,
-				 <&gcc GCC_PCIE3_AHB_CLK>,
-				 <&gcc GCC_PCIE3_PIPE_CLK>;
-			clock-names = "aux", "cfg_ahb", "pipe";
-
-			assigned-clocks = <&gcc GCC_PCIE3_AUX_CLK>;
-			assigned-clock-rates = <20000000>;
-
-			resets = <&gcc GCC_PCIE3_PHY_BCR>,
-				 <&gcc GCC_PCIE3PHY_PHY_BCR>;
-			reset-names = "phy", "common";
-
-			#clock-cells = <0>;
-			clock-output-names = "gcc_pcie3_pipe_clk_src";
-
-			#phy-cells = <0>;
-			status = "disabled";
-		};
-
-		pcie1_phy: phy@fc000 {
-			compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
-			reg = <0x000fc000 0x1000>;
-
-			clocks = <&gcc GCC_PCIE1_AUX_CLK>,
-				 <&gcc GCC_PCIE1_AHB_CLK>,
-				 <&gcc GCC_PCIE1_PIPE_CLK>;
-			clock-names = "aux", "cfg_ahb", "pipe";
-
-			assigned-clocks = <&gcc GCC_PCIE1_AUX_CLK>;
-			assigned-clock-rates = <20000000>;
-
-			resets = <&gcc GCC_PCIE1_PHY_BCR>,
-				 <&gcc GCC_PCIE1PHY_PHY_BCR>;
-			reset-names = "phy", "common";
-
-			#clock-cells = <0>;
-			clock-output-names = "gcc_pcie1_pipe_clk_src";
-
-			#phy-cells = <0>;
-			status = "disabled";
-		};
-
 		cmn_pll: clock-controller@9b000 {
 			compatible = "qcom,ipq9574-cmn-pll";
 			reg = <0x0009b000 0x800>;
@@ -372,48 +486,269 @@ cpu_speed_bin: cpu-speed-bin@15 {
 			};
 		};
 
-		cryptobam: dma-controller@704000 {
-			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
-			reg = <0x00704000 0x20000>;
-			interrupts = <GIC_SPI 207 IRQ_TYPE_LEVEL_HIGH>;
-			#dma-cells = <1>;
-			qcom,ee = <1>;
-			qcom,controlled-remotely;
-		};
-
-		crypto: crypto@73a000 {
-			compatible = "qcom,ipq9574-qce", "qcom,ipq4019-qce", "qcom,qce";
-			reg = <0x0073a000 0x6000>;
-			clocks = <&gcc GCC_CRYPTO_AHB_CLK>,
-				 <&gcc GCC_CRYPTO_AXI_CLK>,
-				 <&gcc GCC_CRYPTO_CLK>;
-			clock-names = "iface", "bus", "core";
-			dmas = <&cryptobam 2>, <&cryptobam 3>;
-			dma-names = "rx", "tx";
+		rng: rng@e3000 {
+			compatible = "qcom,ipq9574-trng", "qcom,trng";
+			reg = <0x000e3000 0x1000>;
+			clocks = <&gcc GCC_PRNG_AHB_CLK>;
+			clock-names = "core";
 		};
 
-		tsens: thermal-sensor@4a9000 {
-			compatible = "qcom,ipq9574-tsens", "qcom,ipq8074-tsens";
-			reg = <0x004a9000 0x1000>,
-			      <0x004a8000 0x1000>;
-			interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-names = "combined";
-			#qcom,sensors = <16>;
-			#thermal-sensor-cells = <1>;
-		};
+		pcie3: pcie@f0000 {
+			compatible = "qcom,pcie-ipq9574";
+			reg = <0x000f0000 0x4000>,
+			      <0x18000000 0xf1d>,
+			      <0x18000f20 0xa8>,
+			      <0x18001000 0x1000>,
+			      <0x18100000 0x1000>;
+			reg-names = "parf",
+				    "dbi",
+				    "elbi",
+				    "atu",
+				    "config";
+			device_type = "pci";
+			linux,pci-domain = <3>;
+			bus-range = <0x00 0xff>;
+			num-lanes = <2>;
+			#address-cells = <3>;
+			#size-cells = <2>;
 
-		tlmm: pinctrl@1000000 {
-			compatible = "qcom,ipq9574-tlmm";
-			reg = <0x01000000 0x300000>;
-			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
-			gpio-controller;
-			#gpio-cells = <2>;
-			gpio-ranges = <&tlmm 0 0 65>;
-			interrupt-controller;
-			#interrupt-cells = <2>;
+			ranges = <0x01000000 0x0 0x00000000 0x18200000 0x0 0x100000>,
+				 <0x02000000 0x0 0x18300000 0x18300000 0x0 0x7d00000>;
 
-			uart2_pins: uart2-state {
-				pins = "gpio34", "gpio35";
+			interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 142 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi0",
+					  "msi1",
+					  "msi2",
+					  "msi3",
+					  "msi4",
+					  "msi5",
+					  "msi6",
+					  "msi7";
+
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 0x7>;
+			interrupt-map = <0 0 0 1 &intc 0 0 189 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &intc 0 0 190 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &intc 0 0 191 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &intc 0 0 192 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&gcc GCC_PCIE3_AXI_M_CLK>,
+				 <&gcc GCC_PCIE3_AXI_S_CLK>,
+				 <&gcc GCC_PCIE3_AXI_S_BRIDGE_CLK>,
+				 <&gcc GCC_PCIE3_RCHNG_CLK>,
+				 <&gcc GCC_PCIE3_AHB_CLK>,
+				 <&gcc GCC_PCIE3_AUX_CLK>;
+			clock-names = "axi_m",
+				      "axi_s",
+				      "axi_bridge",
+				      "rchng",
+				      "ahb",
+				      "aux";
+
+			resets = <&gcc GCC_PCIE3_PIPE_ARES>,
+				 <&gcc GCC_PCIE3_CORE_STICKY_ARES>,
+				 <&gcc GCC_PCIE3_AXI_S_STICKY_ARES>,
+				 <&gcc GCC_PCIE3_AXI_S_ARES>,
+				 <&gcc GCC_PCIE3_AXI_M_STICKY_ARES>,
+				 <&gcc GCC_PCIE3_AXI_M_ARES>,
+				 <&gcc GCC_PCIE3_AUX_ARES>,
+				 <&gcc GCC_PCIE3_AHB_ARES>;
+			reset-names = "pipe",
+				      "sticky",
+				      "axi_s_sticky",
+				      "axi_s",
+				      "axi_m_sticky",
+				      "axi_m",
+				      "aux",
+				      "ahb";
+
+			phys = <&pcie3_phy>;
+			phy-names = "pciephy";
+			interconnects = <&gcc MASTER_ANOC_PCIE3 &gcc SLAVE_ANOC_PCIE3>,
+					<&gcc MASTER_SNOC_PCIE3 &gcc SLAVE_SNOC_PCIE3>;
+			interconnect-names = "pcie-mem", "cpu-pcie";
+			status = "disabled";
+		};
+
+		pcie3_phy: phy@f4000 {
+			compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
+			reg = <0x000f4000 0x2000>;
+
+			clocks = <&gcc GCC_PCIE3_AUX_CLK>,
+				 <&gcc GCC_PCIE3_AHB_CLK>,
+				 <&gcc GCC_PCIE3_PIPE_CLK>;
+			clock-names = "aux", "cfg_ahb", "pipe";
+
+			assigned-clocks = <&gcc GCC_PCIE3_AUX_CLK>;
+			assigned-clock-rates = <20000000>;
+
+			resets = <&gcc GCC_PCIE3_PHY_BCR>,
+				 <&gcc GCC_PCIE3PHY_PHY_BCR>;
+			reset-names = "phy", "common";
+
+			#clock-cells = <0>;
+			clock-output-names = "gcc_pcie3_pipe_clk_src";
+
+			#phy-cells = <0>;
+			status = "disabled";
+		};
+
+		pcie1: pcie@f8000 {
+			compatible = "qcom,pcie-ipq9574";
+			reg = <0x000f8000 0x4000>,
+			      <0x10000000 0xf1d>,
+			      <0x10000f20 0xa8>,
+			      <0x10001000 0x1000>,
+			      <0x10100000 0x1000>;
+			reg-names = "parf",
+				    "dbi",
+				    "elbi",
+				    "atu",
+				    "config";
+			device_type = "pci";
+			linux,pci-domain = <1>;
+			bus-range = <0x00 0xff>;
+			num-lanes = <1>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+
+			ranges = <0x01000000 0x0 0x00000000 0x10200000 0x0 0x100000>,
+				 <0x02000000 0x0 0x10300000 0x10300000 0x0 0x7d00000>;
+
+			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi0",
+					  "msi1",
+					  "msi2",
+					  "msi3",
+					  "msi4",
+					  "msi5",
+					  "msi6",
+					  "msi7";
+
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 0x7>;
+			interrupt-map = <0 0 0 1 &intc 0 0 35 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &intc 0 0 49 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &intc 0 0 84 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &intc 0 0 85 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&gcc GCC_PCIE1_AXI_M_CLK>,
+				 <&gcc GCC_PCIE1_AXI_S_CLK>,
+				 <&gcc GCC_PCIE1_AXI_S_BRIDGE_CLK>,
+				 <&gcc GCC_PCIE1_RCHNG_CLK>,
+				 <&gcc GCC_PCIE1_AHB_CLK>,
+				 <&gcc GCC_PCIE1_AUX_CLK>;
+			clock-names = "axi_m",
+				      "axi_s",
+				      "axi_bridge",
+				      "rchng",
+				      "ahb",
+				      "aux";
+
+			resets = <&gcc GCC_PCIE1_PIPE_ARES>,
+				 <&gcc GCC_PCIE1_CORE_STICKY_ARES>,
+				 <&gcc GCC_PCIE1_AXI_S_STICKY_ARES>,
+				 <&gcc GCC_PCIE1_AXI_S_ARES>,
+				 <&gcc GCC_PCIE1_AXI_M_STICKY_ARES>,
+				 <&gcc GCC_PCIE1_AXI_M_ARES>,
+				 <&gcc GCC_PCIE1_AUX_ARES>,
+				 <&gcc GCC_PCIE1_AHB_ARES>;
+			reset-names = "pipe",
+				      "sticky",
+				      "axi_s_sticky",
+				      "axi_s",
+				      "axi_m_sticky",
+				      "axi_m",
+				      "aux",
+				      "ahb";
+
+			phys = <&pcie1_phy>;
+			phy-names = "pciephy";
+			interconnects = <&gcc MASTER_ANOC_PCIE1 &gcc SLAVE_ANOC_PCIE1>,
+					<&gcc MASTER_SNOC_PCIE1 &gcc SLAVE_SNOC_PCIE1>;
+			interconnect-names = "pcie-mem", "cpu-pcie";
+			status = "disabled";
+		};
+
+		pcie1_phy: phy@fc000 {
+			compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
+			reg = <0x000fc000 0x1000>;
+
+			clocks = <&gcc GCC_PCIE1_AUX_CLK>,
+				 <&gcc GCC_PCIE1_AHB_CLK>,
+				 <&gcc GCC_PCIE1_PIPE_CLK>;
+			clock-names = "aux", "cfg_ahb", "pipe";
+
+			assigned-clocks = <&gcc GCC_PCIE1_AUX_CLK>;
+			assigned-clock-rates = <20000000>;
+
+			resets = <&gcc GCC_PCIE1_PHY_BCR>,
+				 <&gcc GCC_PCIE1PHY_PHY_BCR>;
+			reset-names = "phy", "common";
+
+			#clock-cells = <0>;
+			clock-output-names = "gcc_pcie1_pipe_clk_src";
+
+			#phy-cells = <0>;
+			status = "disabled";
+		};
+
+		cryptobam: dma-controller@704000 {
+			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
+			reg = <0x00704000 0x20000>;
+			interrupts = <GIC_SPI 207 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			qcom,ee = <1>;
+			qcom,controlled-remotely;
+		};
+
+		crypto: crypto@73a000 {
+			compatible = "qcom,ipq9574-qce", "qcom,ipq4019-qce", "qcom,qce";
+			reg = <0x0073a000 0x6000>;
+			clocks = <&gcc GCC_CRYPTO_AHB_CLK>,
+				 <&gcc GCC_CRYPTO_AXI_CLK>,
+				 <&gcc GCC_CRYPTO_CLK>;
+			clock-names = "iface", "bus", "core";
+			dmas = <&cryptobam 2>, <&cryptobam 3>;
+			dma-names = "rx", "tx";
+		};
+
+		tsens: thermal-sensor@4a9000 {
+			compatible = "qcom,ipq9574-tsens", "qcom,ipq8074-tsens";
+			reg = <0x004a9000 0x1000>,
+			      <0x004a8000 0x1000>;
+			interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "combined";
+			#qcom,sensors = <16>;
+			#thermal-sensor-cells = <1>;
+		};
+
+		tlmm: pinctrl@1000000 {
+			compatible = "qcom,ipq9574-tlmm";
+			reg = <0x01000000 0x300000>;
+			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&tlmm 0 0 65>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+
+			uart2_pins: uart2-state {
+				pins = "gpio34", "gpio35";
 				function = "blsp2_uart";
 				drive-strength = <8>;
 				bias-disable;
@@ -873,326 +1208,6 @@ frame@b128000 {
 				status = "disabled";
 			};
 		};
-
-		pcie1: pcie@10000000 {
-			compatible = "qcom,pcie-ipq9574";
-			reg =  <0x10000000 0xf1d>,
-			       <0x10000f20 0xa8>,
-			       <0x10001000 0x1000>,
-			       <0x000f8000 0x4000>,
-			       <0x10100000 0x1000>;
-			reg-names = "dbi", "elbi", "atu", "parf", "config";
-			device_type = "pci";
-			linux,pci-domain = <1>;
-			bus-range = <0x00 0xff>;
-			num-lanes = <1>;
-			#address-cells = <3>;
-			#size-cells = <2>;
-
-			ranges = <0x01000000 0x0 0x00000000 0x10200000 0x0 0x100000>,
-				 <0x02000000 0x0 0x10300000 0x10300000 0x0 0x7d00000>;
-
-			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-names = "msi0",
-					  "msi1",
-					  "msi2",
-					  "msi3",
-					  "msi4",
-					  "msi5",
-					  "msi6",
-					  "msi7";
-
-			#interrupt-cells = <1>;
-			interrupt-map-mask = <0 0 0 0x7>;
-			interrupt-map = <0 0 0 1 &intc 0 0 35 IRQ_TYPE_LEVEL_HIGH>,
-					<0 0 0 2 &intc 0 0 49 IRQ_TYPE_LEVEL_HIGH>,
-					<0 0 0 3 &intc 0 0 84 IRQ_TYPE_LEVEL_HIGH>,
-					<0 0 0 4 &intc 0 0 85 IRQ_TYPE_LEVEL_HIGH>;
-
-			clocks = <&gcc GCC_PCIE1_AXI_M_CLK>,
-				 <&gcc GCC_PCIE1_AXI_S_CLK>,
-				 <&gcc GCC_PCIE1_AXI_S_BRIDGE_CLK>,
-				 <&gcc GCC_PCIE1_RCHNG_CLK>,
-				 <&gcc GCC_PCIE1_AHB_CLK>,
-				 <&gcc GCC_PCIE1_AUX_CLK>;
-			clock-names = "axi_m",
-				      "axi_s",
-				      "axi_bridge",
-				      "rchng",
-				      "ahb",
-				      "aux";
-
-			resets = <&gcc GCC_PCIE1_PIPE_ARES>,
-				 <&gcc GCC_PCIE1_CORE_STICKY_ARES>,
-				 <&gcc GCC_PCIE1_AXI_S_STICKY_ARES>,
-				 <&gcc GCC_PCIE1_AXI_S_ARES>,
-				 <&gcc GCC_PCIE1_AXI_M_STICKY_ARES>,
-				 <&gcc GCC_PCIE1_AXI_M_ARES>,
-				 <&gcc GCC_PCIE1_AUX_ARES>,
-				 <&gcc GCC_PCIE1_AHB_ARES>;
-			reset-names = "pipe",
-				      "sticky",
-				      "axi_s_sticky",
-				      "axi_s",
-				      "axi_m_sticky",
-				      "axi_m",
-				      "aux",
-				      "ahb";
-
-			phys = <&pcie1_phy>;
-			phy-names = "pciephy";
-			interconnects = <&gcc MASTER_ANOC_PCIE1 &gcc SLAVE_ANOC_PCIE1>,
-					<&gcc MASTER_SNOC_PCIE1 &gcc SLAVE_SNOC_PCIE1>;
-			interconnect-names = "pcie-mem", "cpu-pcie";
-			status = "disabled";
-		};
-
-		pcie3: pcie@18000000 {
-			compatible = "qcom,pcie-ipq9574";
-			reg =  <0x18000000 0xf1d>,
-			       <0x18000f20 0xa8>,
-			       <0x18001000 0x1000>,
-			       <0x000f0000 0x4000>,
-			       <0x18100000 0x1000>;
-			reg-names = "dbi", "elbi", "atu", "parf", "config";
-			device_type = "pci";
-			linux,pci-domain = <3>;
-			bus-range = <0x00 0xff>;
-			num-lanes = <2>;
-			#address-cells = <3>;
-			#size-cells = <2>;
-
-			ranges = <0x01000000 0x0 0x00000000 0x18200000 0x0 0x100000>,
-				 <0x02000000 0x0 0x18300000 0x18300000 0x0 0x7d00000>;
-
-			interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 142 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-names = "msi0",
-					  "msi1",
-					  "msi2",
-					  "msi3",
-					  "msi4",
-					  "msi5",
-					  "msi6",
-					  "msi7";
-
-			#interrupt-cells = <1>;
-			interrupt-map-mask = <0 0 0 0x7>;
-			interrupt-map = <0 0 0 1 &intc 0 0 189 IRQ_TYPE_LEVEL_HIGH>,
-					<0 0 0 2 &intc 0 0 190 IRQ_TYPE_LEVEL_HIGH>,
-					<0 0 0 3 &intc 0 0 191 IRQ_TYPE_LEVEL_HIGH>,
-					<0 0 0 4 &intc 0 0 192 IRQ_TYPE_LEVEL_HIGH>;
-
-			clocks = <&gcc GCC_PCIE3_AXI_M_CLK>,
-				 <&gcc GCC_PCIE3_AXI_S_CLK>,
-				 <&gcc GCC_PCIE3_AXI_S_BRIDGE_CLK>,
-				 <&gcc GCC_PCIE3_RCHNG_CLK>,
-				 <&gcc GCC_PCIE3_AHB_CLK>,
-				 <&gcc GCC_PCIE3_AUX_CLK>;
-			clock-names = "axi_m",
-				      "axi_s",
-				      "axi_bridge",
-				      "rchng",
-				      "ahb",
-				      "aux";
-
-			resets = <&gcc GCC_PCIE3_PIPE_ARES>,
-				 <&gcc GCC_PCIE3_CORE_STICKY_ARES>,
-				 <&gcc GCC_PCIE3_AXI_S_STICKY_ARES>,
-				 <&gcc GCC_PCIE3_AXI_S_ARES>,
-				 <&gcc GCC_PCIE3_AXI_M_STICKY_ARES>,
-				 <&gcc GCC_PCIE3_AXI_M_ARES>,
-				 <&gcc GCC_PCIE3_AUX_ARES>,
-				 <&gcc GCC_PCIE3_AHB_ARES>;
-			reset-names = "pipe",
-				      "sticky",
-				      "axi_s_sticky",
-				      "axi_s",
-				      "axi_m_sticky",
-				      "axi_m",
-				      "aux",
-				      "ahb";
-
-			phys = <&pcie3_phy>;
-			phy-names = "pciephy";
-			interconnects = <&gcc MASTER_ANOC_PCIE3 &gcc SLAVE_ANOC_PCIE3>,
-					<&gcc MASTER_SNOC_PCIE3 &gcc SLAVE_SNOC_PCIE3>;
-			interconnect-names = "pcie-mem", "cpu-pcie";
-			status = "disabled";
-		};
-
-		pcie2: pcie@20000000 {
-			compatible = "qcom,pcie-ipq9574";
-			reg =  <0x20000000 0xf1d>,
-			       <0x20000f20 0xa8>,
-			       <0x20001000 0x1000>,
-			       <0x00088000 0x4000>,
-			       <0x20100000 0x1000>;
-			reg-names = "dbi", "elbi", "atu", "parf", "config";
-			device_type = "pci";
-			linux,pci-domain = <2>;
-			bus-range = <0x00 0xff>;
-			num-lanes = <2>;
-			#address-cells = <3>;
-			#size-cells = <2>;
-
-			ranges = <0x01000000 0x0 0x00000000 0x20200000 0x0 0x100000>,
-				 <0x02000000 0x0 0x20300000 0x20300000 0x0 0x7d00000>;
-
-			interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 142 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-names = "msi0",
-					  "msi1",
-					  "msi2",
-					  "msi3",
-					  "msi4",
-					  "msi5",
-					  "msi6",
-					  "msi7";
-
-			#interrupt-cells = <1>;
-			interrupt-map-mask = <0 0 0 0x7>;
-			interrupt-map = <0 0 0 1 &intc 0 0 164 IRQ_TYPE_LEVEL_HIGH>,
-					<0 0 0 2 &intc 0 0 165 IRQ_TYPE_LEVEL_HIGH>,
-					<0 0 0 3 &intc 0 0 186 IRQ_TYPE_LEVEL_HIGH>,
-					<0 0 0 4 &intc 0 0 187 IRQ_TYPE_LEVEL_HIGH>;
-
-			clocks = <&gcc GCC_PCIE2_AXI_M_CLK>,
-				 <&gcc GCC_PCIE2_AXI_S_CLK>,
-				 <&gcc GCC_PCIE2_AXI_S_BRIDGE_CLK>,
-				 <&gcc GCC_PCIE2_RCHNG_CLK>,
-				 <&gcc GCC_PCIE2_AHB_CLK>,
-				 <&gcc GCC_PCIE2_AUX_CLK>;
-			clock-names = "axi_m",
-				      "axi_s",
-				      "axi_bridge",
-				      "rchng",
-				      "ahb",
-				      "aux";
-
-			resets = <&gcc GCC_PCIE2_PIPE_ARES>,
-				 <&gcc GCC_PCIE2_CORE_STICKY_ARES>,
-				 <&gcc GCC_PCIE2_AXI_S_STICKY_ARES>,
-				 <&gcc GCC_PCIE2_AXI_S_ARES>,
-				 <&gcc GCC_PCIE2_AXI_M_STICKY_ARES>,
-				 <&gcc GCC_PCIE2_AXI_M_ARES>,
-				 <&gcc GCC_PCIE2_AUX_ARES>,
-				 <&gcc GCC_PCIE2_AHB_ARES>;
-			reset-names = "pipe",
-				      "sticky",
-				      "axi_s_sticky",
-				      "axi_s",
-				      "axi_m_sticky",
-				      "axi_m",
-				      "aux",
-				      "ahb";
-
-			phys = <&pcie2_phy>;
-			phy-names = "pciephy";
-			interconnects = <&gcc MASTER_ANOC_PCIE2 &gcc SLAVE_ANOC_PCIE2>,
-					<&gcc MASTER_SNOC_PCIE2 &gcc SLAVE_SNOC_PCIE2>;
-			interconnect-names = "pcie-mem", "cpu-pcie";
-			status = "disabled";
-		};
-
-		pcie0: pci@28000000 {
-			compatible = "qcom,pcie-ipq9574";
-			reg =  <0x28000000 0xf1d>,
-			       <0x28000f20 0xa8>,
-			       <0x28001000 0x1000>,
-			       <0x00080000 0x4000>,
-			       <0x28100000 0x1000>;
-			reg-names = "dbi", "elbi", "atu", "parf", "config";
-			device_type = "pci";
-			linux,pci-domain = <0>;
-			bus-range = <0x00 0xff>;
-			num-lanes = <1>;
-			#address-cells = <3>;
-			#size-cells = <2>;
-
-			ranges = <0x01000000 0x0 0x00000000 0x28200000 0x0 0x100000>,
-				 <0x02000000 0x0 0x28300000 0x28300000 0x0 0x7d00000>;
-			interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-names = "msi0",
-					  "msi1",
-					  "msi2",
-					  "msi3",
-					  "msi4",
-					  "msi5",
-					  "msi6",
-					  "msi7";
-
-			#interrupt-cells = <1>;
-			interrupt-map-mask = <0 0 0 0x7>;
-			interrupt-map = <0 0 0 1 &intc 0 0 75 IRQ_TYPE_LEVEL_HIGH>,
-					<0 0 0 2 &intc 0 0 78 IRQ_TYPE_LEVEL_HIGH>,
-					<0 0 0 3 &intc 0 0 79 IRQ_TYPE_LEVEL_HIGH>,
-					<0 0 0 4 &intc 0 0 83 IRQ_TYPE_LEVEL_HIGH>;
-
-			clocks = <&gcc GCC_PCIE0_AXI_M_CLK>,
-				 <&gcc GCC_PCIE0_AXI_S_CLK>,
-				 <&gcc GCC_PCIE0_AXI_S_BRIDGE_CLK>,
-				 <&gcc GCC_PCIE0_RCHNG_CLK>,
-				 <&gcc GCC_PCIE0_AHB_CLK>,
-				 <&gcc GCC_PCIE0_AUX_CLK>;
-			clock-names = "axi_m",
-				      "axi_s",
-				      "axi_bridge",
-				      "rchng",
-				      "ahb",
-				      "aux";
-
-			resets = <&gcc GCC_PCIE0_PIPE_ARES>,
-				 <&gcc GCC_PCIE0_CORE_STICKY_ARES>,
-				 <&gcc GCC_PCIE0_AXI_S_STICKY_ARES>,
-				 <&gcc GCC_PCIE0_AXI_S_ARES>,
-				 <&gcc GCC_PCIE0_AXI_M_STICKY_ARES>,
-				 <&gcc GCC_PCIE0_AXI_M_ARES>,
-				 <&gcc GCC_PCIE0_AUX_ARES>,
-				 <&gcc GCC_PCIE0_AHB_ARES>;
-			reset-names = "pipe",
-				      "sticky",
-				      "axi_s_sticky",
-				      "axi_s",
-				      "axi_m_sticky",
-				      "axi_m",
-				      "aux",
-				      "ahb";
-
-			phys = <&pcie0_phy>;
-			phy-names = "pciephy";
-			interconnects = <&gcc MASTER_ANOC_PCIE0 &gcc SLAVE_ANOC_PCIE0>,
-					<&gcc MASTER_SNOC_PCIE0 &gcc SLAVE_SNOC_PCIE0>;
-			interconnect-names = "pcie-mem", "cpu-pcie";
-			status = "disabled";
-		};
-
 	};
 
 	thermal-zones {
-- 
2.34.1

[PATCH v11 5/7] dt-bindings: PCI: qcom: Document the IPQ5332 PCIe controller

[Varadarajan Narayanan]

Document the PCIe controller on IPQ5332 platform. IPQ5332 will use IPQ9574
as the fall back compatible.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v11: Add 'Reviewed-by: Krzysztof Kozlowski'

v10: Remove unnecessary ipq5332 constraint in 'power domains' not required
     constraint
     Fix maxItems for interrupts contstraint of sdm845

v9: Remove superfluous ipq5332 constraint since the fallback is present

v8: Use ipq9574 as fallback compatible for ipq5332 along with ipq5424

v7: Moved ipq9574 related changes to a separate patch
    Add 'global' interrupt

v6: Commit message update only. Add info regarding the moving of
    ipq9574 from 5 "reg" definition to 5 or 6 reg definition.

v5: Re-arrange 5332 and 9574 compatibles to handle fallback usage in dts

v4: * v3 reused ipq9574 bindings for ipq5332. Instead add one for ipq5332
    * DTS uses ipq9574 compatible as fallback. Hence move ipq9574 to be able
      to use the 'reg' section for both ipq5332 and ipq9574. Else, dtbs_check
      and dt_binding_check flag errors.
---
 Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 4b4927178abc..6696a36009da 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -33,6 +33,7 @@ properties:
           - qcom,pcie-sdx55
       - items:
           - enum:
+              - qcom,pcie-ipq5332
               - qcom,pcie-ipq5424
           - const: qcom,pcie-ipq9574
       - items:
@@ -49,11 +50,11 @@ properties:
 
   interrupts:
     minItems: 1
-    maxItems: 8
+    maxItems: 9
 
   interrupt-names:
     minItems: 1
-    maxItems: 8
+    maxItems: 9
 
   iommu-map:
     minItems: 1
@@ -443,6 +444,7 @@ allOf:
         interrupts:
           minItems: 8
         interrupt-names:
+          minItems: 8
           items:
             - const: msi0
             - const: msi1
@@ -452,6 +454,7 @@ allOf:
             - const: msi5
             - const: msi6
             - const: msi7
+            - const: global
 
   - if:
       properties:
@@ -599,6 +602,7 @@ allOf:
         - properties:
             interrupts:
               minItems: 8
+              maxItems: 8
             interrupt-names:
               items:
                 - const: msi0
-- 
2.34.1

[PATCH v11 6/7] arm64: dts: qcom: ipq5332: Add PCIe related nodes

[Varadarajan Narayanan]

From: Praveenkumar I <quic_ipkumar@quicinc.com>

Add phy and controller nodes for pcie0_x1 and pcie1_x2.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v10: Trim down the list of assigned clocks
     Fix arch/arm64/boot/dts/qcom/ipq5332.dtsi:627.24-729.5: Warning (simple_bus_reg): /soc@0/pcie@20000000: simple-bus unit address format error, expected "80000"
     Rearrange nodes w.r.t. address sort order

v7: * Fix IO 'ranges' entry
    * Add root port definitions
    * Not adding 'dma-coherent' as the controller doesn't have that support
    * Remove 'bus-range' as it has default values
    * Group root complex related entries and root port related entries
      separately

v6: * Add 'num-lanes' to "pcie0_phy: phy@4b0000"
    * Earlier, some related clock rates were set in U-Boot. In
      recent versions of U-Boot this has been removed resulting
      in the phy link not coming up. To remove boot loader
      dependency add assigned-clocks and assigned-clock-rates to
      the controller nodes.
    * Not sure if 'Reviewed-by' should be dropped.

v5: Add 'num-lanes' to "pcie1_phy: phy@4b1000"
    Make ipq5332 as main and ipq9574 as fallback compatible
    Move controller nodes per address
    Having Konrad's Reviewed-By

v4: Remove 'reset-names' as driver uses bulk APIs
    Remove 'clock-output-names' as driver uses bulk APIs
    Add missing reset for pcie1_phy
    Convert 'reg-names' to a vertical list
    Move 'msi-map' before interrupts

v3: Fix compatible string for phy nodes
    Use ipq9574 as backup compatible instead of new compatible for ipq5332
    Fix mixed case hex addresses
    Add "mhi" space
    Removed unnecessary comments and stray blank lines

v2: Fix nodes' location per address
---
 arch/arm64/boot/dts/qcom/ipq5332.dtsi | 252 +++++++++++++++++++++++++-
 1 file changed, 250 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index ca3da95730bd..1115d8388ef9 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -167,6 +167,214 @@ usbphy0: phy@7b000 {
 			status = "disabled";
 		};
 
+		pcie0: pcie@80000 {
+			compatible = "qcom,pcie-ipq5332", "qcom,pcie-ipq9574";
+			reg = <0x00080000 0x3000>,
+			      <0x20000000 0xf1d>,
+			      <0x20000f20 0xa8>,
+			      <0x20001000 0x1000>,
+			      <0x20100000 0x1000>,
+			      <0x00083000 0x1000>;
+			reg-names = "parf",
+				    "dbi",
+				    "elbi",
+				    "atu",
+				    "config",
+				    "mhi";
+			device_type = "pci";
+			linux,pci-domain = <0>;
+			num-lanes = <1>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+
+			ranges = <0x01000000 0x0 0x00000000 0x20200000 0x0 0x00100000>,
+				 <0x02000000 0x0 0x20300000 0x20300000 0x0 0x0fd00000>;
+
+			msi-map = <0x0 &v2m0 0x0 0xffd>;
+
+			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi0",
+					  "msi1",
+					  "msi2",
+					  "msi3",
+					  "msi4",
+					  "msi5",
+					  "msi6",
+					  "msi7",
+					  "global";
+
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 0x7>;
+			interrupt-map = <0 0 0 1 &intc 0 0 35 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &intc 0 0 36 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &intc 0 0 37 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &intc 0 0 38 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&gcc GCC_PCIE3X1_0_AXI_M_CLK>,
+				 <&gcc GCC_PCIE3X1_0_AXI_S_CLK>,
+				 <&gcc GCC_PCIE3X1_0_AXI_S_BRIDGE_CLK>,
+				 <&gcc GCC_PCIE3X1_0_RCHG_CLK>,
+				 <&gcc GCC_PCIE3X1_0_AHB_CLK>,
+				 <&gcc GCC_PCIE3X1_0_AUX_CLK>;
+			clock-names = "axi_m",
+				      "axi_s",
+				      "axi_bridge",
+				      "rchng",
+				      "ahb",
+				      "aux";
+
+			assigned-clocks = <&gcc GCC_PCIE3X1_0_AUX_CLK>;
+
+			assigned-clock-rates = <2000000>;
+
+			resets = <&gcc GCC_PCIE3X1_0_PIPE_ARES>,
+				 <&gcc GCC_PCIE3X1_0_CORE_STICKY_ARES>,
+				 <&gcc GCC_PCIE3X1_0_AXI_S_STICKY_ARES>,
+				 <&gcc GCC_PCIE3X1_0_AXI_S_CLK_ARES>,
+				 <&gcc GCC_PCIE3X1_0_AXI_M_STICKY_ARES>,
+				 <&gcc GCC_PCIE3X1_0_AXI_M_CLK_ARES>,
+				 <&gcc GCC_PCIE3X1_0_AUX_CLK_ARES>,
+				 <&gcc GCC_PCIE3X1_0_AHB_CLK_ARES>;
+			reset-names = "pipe",
+				      "sticky",
+				      "axi_s_sticky",
+				      "axi_s",
+				      "axi_m_sticky",
+				      "axi_m",
+				      "aux",
+				      "ahb";
+
+			phys = <&pcie0_phy>;
+			phy-names = "pciephy";
+
+			interconnects = <&gcc MASTER_SNOC_PCIE3_1_M &gcc SLAVE_SNOC_PCIE3_1_M>,
+					<&gcc MASTER_ANOC_PCIE3_1_S &gcc SLAVE_ANOC_PCIE3_1_S>;
+			interconnect-names = "pcie-mem", "cpu-pcie";
+
+			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
+		};
+
+		pcie1: pcie@88000 {
+			compatible = "qcom,pcie-ipq5332", "qcom,pcie-ipq9574";
+			reg = <0x00088000 0x3000>,
+			      <0x18000000 0xf1d>,
+			      <0x18000f20 0xa8>,
+			      <0x18001000 0x1000>,
+			      <0x18100000 0x1000>,
+			      <0x0008b000 0x1000>;
+			reg-names = "parf",
+				    "dbi",
+				    "elbi",
+				    "atu",
+				    "config",
+				    "mhi";
+			device_type = "pci";
+			linux,pci-domain = <1>;
+			num-lanes = <2>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+
+			ranges = <0x01000000 0x0 0x00000000 0x18200000 0x0 0x00100000>,
+				 <0x02000000 0x0 0x18300000 0x18300000 0x0 0x07d00000>;
+
+			msi-map = <0x0 &v2m0 0x0 0xffd>;
+
+			interrupts = <GIC_SPI 403 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 404 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 405 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 407 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 408 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 409 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 411 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi0",
+					  "msi1",
+					  "msi2",
+					  "msi3",
+					  "msi4",
+					  "msi5",
+					  "msi6",
+					  "msi7",
+					  "global";
+
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 0x7>;
+			interrupt-map = <0 0 0 1 &intc 0 0 412 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &intc 0 0 413 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &intc 0 0 414 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &intc 0 0 415 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&gcc GCC_PCIE3X2_AXI_M_CLK>,
+				 <&gcc GCC_PCIE3X2_AXI_S_CLK>,
+				 <&gcc GCC_PCIE3X2_AXI_S_BRIDGE_CLK>,
+				 <&gcc GCC_PCIE3X2_RCHG_CLK>,
+				 <&gcc GCC_PCIE3X2_AHB_CLK>,
+				 <&gcc GCC_PCIE3X2_AUX_CLK>;
+			clock-names = "axi_m",
+				      "axi_s",
+				      "axi_bridge",
+				      "rchng",
+				      "ahb",
+				      "aux";
+
+			assigned-clocks = <&gcc GCC_PCIE3X2_AUX_CLK>;
+
+			assigned-clock-rates = <2000000>;
+
+			resets = <&gcc GCC_PCIE3X2_PIPE_ARES>,
+				 <&gcc GCC_PCIE3X2_CORE_STICKY_ARES>,
+				 <&gcc GCC_PCIE3X2_AXI_S_STICKY_ARES>,
+				 <&gcc GCC_PCIE3X2_AXI_S_CLK_ARES>,
+				 <&gcc GCC_PCIE3X2_AXI_M_STICKY_ARES>,
+				 <&gcc GCC_PCIE3X2_AXI_M_CLK_ARES>,
+				 <&gcc GCC_PCIE3X2_AUX_CLK_ARES>,
+				 <&gcc GCC_PCIE3X2_AHB_CLK_ARES>;
+			reset-names = "pipe",
+				      "sticky",
+				      "axi_s_sticky",
+				      "axi_s",
+				      "axi_m_sticky",
+				      "axi_m",
+				      "aux",
+				      "ahb";
+
+			phys = <&pcie1_phy>;
+			phy-names = "pciephy";
+
+			interconnects = <&gcc MASTER_SNOC_PCIE3_2_M &gcc SLAVE_SNOC_PCIE3_2_M>,
+					<&gcc MASTER_ANOC_PCIE3_2_S &gcc SLAVE_ANOC_PCIE3_2_S>;
+			interconnect-names = "pcie-mem", "cpu-pcie";
+
+			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
+		};
+
 		qfprom: efuse@a4000 {
 			compatible = "qcom,ipq5332-qfprom", "qcom,qfprom";
 			reg = <0x000a4000 0x721>;
@@ -186,6 +394,46 @@ rng: rng@e3000 {
 			clock-names = "core";
 		};
 
+		pcie0_phy: phy@4b0000 {
+			compatible = "qcom,ipq5332-uniphy-pcie-phy";
+			reg = <0x004b0000 0x800>;
+
+			clocks = <&gcc GCC_PCIE3X1_0_PIPE_CLK>,
+				 <&gcc GCC_PCIE3X1_PHY_AHB_CLK>;
+
+			resets = <&gcc GCC_PCIE3X1_0_PHY_BCR>,
+				 <&gcc GCC_PCIE3X1_PHY_AHB_CLK_ARES>,
+				 <&gcc GCC_PCIE3X1_0_PHY_PHY_BCR>;
+
+			#clock-cells = <0>;
+
+			#phy-cells = <0>;
+
+			num-lanes = <1>;
+
+			status = "disabled";
+		};
+
+		pcie1_phy: phy@4b1000 {
+			compatible = "qcom,ipq5332-uniphy-pcie-phy";
+			reg = <0x004b1000 0x1000>;
+
+			clocks = <&gcc GCC_PCIE3X2_PIPE_CLK>,
+				 <&gcc GCC_PCIE3X2_PHY_AHB_CLK>;
+
+			resets = <&gcc GCC_PCIE3X2_PHY_BCR>,
+				 <&gcc GCC_PCIE3X2_PHY_AHB_CLK_ARES>,
+				 <&gcc GCC_PCIE3X2PHY_PHY_BCR>;
+
+			#clock-cells = <0>;
+
+			#phy-cells = <0>;
+
+			num-lanes = <2>;
+
+			status = "disabled";
+		};
+
 		tlmm: pinctrl@1000000 {
 			compatible = "qcom,ipq5332-tlmm";
 			reg = <0x01000000 0x300000>;
@@ -212,8 +460,8 @@ gcc: clock-controller@1800000 {
 			#interconnect-cells = <1>;
 			clocks = <&xo_board>,
 				 <&sleep_clk>,
-				 <0>,
-				 <0>,
+				 <&pcie1_phy>,
+				 <&pcie0_phy>,
 				 <0>;
 		};
 
-- 
2.34.1

[PATCH v11 7/7] arm64: dts: qcom: ipq5332-rdp441: Enable PCIe phys and controllers

[Varadarajan Narayanan]

From: Praveenkumar I <quic_ipkumar@quicinc.com>

Enable the PCIe controller and PHY nodes for RDP 441.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5: Add 'Reviewed-by: Konrad Dybcio'

v4: Fix nodes sort order
    Use property-n followed by property-names

v3: Reorder nodes alphabetically
    Fix commit subject
---
 arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts | 76 +++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
index 846413817e9a..79ec77cfe552 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
@@ -32,6 +32,34 @@ &sdhc {
 	status = "okay";
 };
 
+&pcie0 {
+	pinctrl-0 = <&pcie0_default>;
+	pinctrl-names = "default";
+
+	perst-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 39 GPIO_ACTIVE_LOW>;
+
+	status = "okay";
+};
+
+&pcie0_phy {
+	status = "okay";
+};
+
+&pcie1 {
+	pinctrl-0 = <&pcie1_default>;
+	pinctrl-names = "default";
+
+	perst-gpios = <&tlmm 47 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 48 GPIO_ACTIVE_LOW>;
+
+	status = "okay";
+};
+
+&pcie1_phy {
+	status = "okay";
+};
+
 &tlmm {
 	i2c_1_pins: i2c-1-state {
 		pins = "gpio29", "gpio30";
@@ -40,6 +68,54 @@ i2c_1_pins: i2c-1-state {
 		bias-pull-up;
 	};
 
+	pcie0_default: pcie0-default-state {
+		clkreq-n-pins {
+			pins = "gpio37";
+			function = "pcie0_clk";
+			drive-strength = <8>;
+			bias-pull-up;
+		};
+
+		perst-n-pins {
+			pins = "gpio38";
+			function = "gpio";
+			drive-strength = <8>;
+			bias-pull-up;
+			output-low;
+		};
+
+		wake-n-pins {
+			pins = "gpio39";
+			function = "pcie0_wake";
+			drive-strength = <8>;
+			bias-pull-up;
+		};
+	};
+
+	pcie1_default: pcie1-default-state {
+		clkreq-n-pins {
+			pins = "gpio46";
+			function = "pcie1_clk";
+			drive-strength = <8>;
+			bias-pull-up;
+		};
+
+		perst-n-pins {
+			pins = "gpio47";
+			function = "gpio";
+			drive-strength = <8>;
+			bias-pull-up;
+			output-low;
+		};
+
+		wake-n-pins {
+			pins = "gpio48";
+			function = "pcie1_wake";
+			drive-strength = <8>;
+			bias-pull-up;
+		};
+	};
+
 	sdc_default_state: sdc-default-state {
 		clk-pins {
 			pins = "gpio13";
-- 
2.34.1

Re: [PATCH v11 0/7] Add PCIe support for Qualcomm IPQ5332

[Krzysztof Wilczyński]

Hello,

> Patch series adds support for enabling the PCIe controller and
> UNIPHY found on Qualcomm IPQ5332 platform. PCIe0 is Gen3 X1 and
> PCIe1 is Gen3 X2 are added.

Applied to dt-bindings, thank you!

	Krzysztof

Re: [PATCH v11 4/7] arm64: dts: qcom: ipq9574: Reorder reg and reg-names

[Krzysztof Kozlowski]

On 20/02/2025 10:42, Varadarajan Narayanan wrote:
> The 'reg' & 'reg-names' constraints used in the bindings and dtsi are
> different resulting in dt_bindings_check errors. Re-order the reg entries,

Why?

> fix the node names and move the nodes to maintain sort order to address the

Fixing (how?) node name looks like separate problem.


> following errors/warnings.
> 
> 	arch/arm64/boot/dts/qcom/ipq9574-rdp449.dtb: pcie@20000000: reg-names:0: 'parf' was expected

So this was added back in 2024 and never tested?

> 	arch/arm64/boot/dts/qcom/ipq9574.dtsi:1045.24-1127.5: Warning (simple_bus_reg): /soc@0/pcie@20000000: simple-bus unit address format error, expected "88000"
> 
> Move the nodes to maintain sort order w.r.t address.
> 

I don't understand this commit msg and huge diff does not help. It's
very difficult to spot the actual changes and since Qualcomm was never
testing this in the past, I do not believe it is being tested now.

Clearly explain what is the problem - *each of them*.

Best regards,
Krzysztof

Re: [PATCH v11 3/7] dt-bindings: PCI: qcom: Use sdx55 reg description for ipq9574

[Krzysztof Kozlowski]

On 20/02/2025 10:42, Varadarajan Narayanan wrote:
> All DT entries except "reg" is similar between ipq5332 and ipq9574. ipq9574
> has 5 registers while ipq5332 has 6. MHI is the additional (i.e. sixth
> entry). Since this matches with the sdx55's "reg" definition which allows
> for 5 or 6 registers, combine ipq9574 with sdx55.
> 
> This change is to prepare ipq9574 to be used as ipq5332's fallback
> compatible.
> 
> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Unreviewed.

> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v8: Add 'Reviewed-by: Krzysztof Kozlowski'
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 7235d6554cfb..4b4927178abc 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -169,7 +169,6 @@ allOf:
>              enum:
>                - qcom,pcie-ipq6018
>                - qcom,pcie-ipq8074-gen3
> -              - qcom,pcie-ipq9574

Why you did not explain that you are going to affect users of DTS?

NAK

Best regards,
Krzysztof

Re: [PATCH v11 4/7] arm64: dts: qcom: ipq9574: Reorder reg and reg-names

[Krzysztof Kozlowski]

On 06/03/2025 12:49, Krzysztof Kozlowski wrote:
> On 20/02/2025 10:42, Varadarajan Narayanan wrote:
>> The 'reg' & 'reg-names' constraints used in the bindings and dtsi are
>> different resulting in dt_bindings_check errors. Re-order the reg entries,
> 
> Why?
> 
>> fix the node names and move the nodes to maintain sort order to address the
> 
> Fixing (how?) node name looks like separate problem.
> 
> 
>> following errors/warnings.
>>
>> 	arch/arm64/boot/dts/qcom/ipq9574-rdp449.dtb: pcie@20000000: reg-names:0: 'parf' was expected

How can I reproduce this error?

Isn't this error which you intentionally added and now you claim you
fix? In the same patchset?

This really looks like breaking things just to call it "look, I fixed
something" two patches later in the same set.

Best regards,
Krzysztof

Re: [PATCH v11 0/7] Add PCIe support for Qualcomm IPQ5332

[Krzysztof Kozlowski]

On 20/02/2025 15:45, Krzysztof Wilczyński wrote:
> Hello,
> 
>> Patch series adds support for enabling the PCIe controller and
>> UNIPHY found on Qualcomm IPQ5332 platform. PCIe0 is Gen3 X1 and
>> PCIe1 is Gen3 X2 are added.
> 
> Applied to dt-bindings, thank you!
I will send reverts for these. This patchset affects users without
mentioning it and without providing any rationale.

What's more, it introduces known to author warnings just to fix them
later...

Best regards,
Krzysztof

Re: [PATCH v11 3/7] dt-bindings: PCI: qcom: Use sdx55 reg description for ipq9574

[Krzysztof Kozlowski]

On 06/03/2025 12:52, Krzysztof Kozlowski wrote:
> On 20/02/2025 10:42, Varadarajan Narayanan wrote:
>> All DT entries except "reg" is similar between ipq5332 and ipq9574. ipq9574
>> has 5 registers while ipq5332 has 6. MHI is the additional (i.e. sixth
>> entry). Since this matches with the sdx55's "reg" definition which allows
>> for 5 or 6 registers, combine ipq9574 with sdx55.
>>
>> This change is to prepare ipq9574 to be used as ipq5332's fallback
>> compatible.
>>
>> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Unreviewed.
> 
>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
>> ---
>> v8: Add 'Reviewed-by: Krzysztof Kozlowski'
>> ---
>>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> index 7235d6554cfb..4b4927178abc 100644
>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> @@ -169,7 +169,6 @@ allOf:
>>              enum:
>>                - qcom,pcie-ipq6018
>>                - qcom,pcie-ipq8074-gen3
>> -              - qcom,pcie-ipq9574
> 
> Why you did not explain that you are going to affect users of DTS?
> 
> NAK

I did not connect the dots, but I pointed out that you break users and
your DTS is wrong:
https://lore.kernel.org/all/f7551daa-cce5-47b3-873f-21b9c5026ed2@kernel.org/

so you should come back with questions to clarify what to do, not keep
pushing this incorrect patchset.

My bad, I should really have zero trust.

Best regards,
Krzysztof

Re: [PATCH v11 6/7] arm64: dts: qcom: ipq5332: Add PCIe related nodes

[Krzysztof Kozlowski]

On 20/02/2025 10:42, Varadarajan Narayanan wrote:
> From: Praveenkumar I <quic_ipkumar@quicinc.com>
> 
> Add phy and controller nodes for pcie0_x1 and pcie1_x2.
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v10: Trim down the list of assigned clocks
>      Fix arch/arm64/boot/dts/qcom/ipq5332.dtsi:627.24-729.5: Warning (simple_bus_reg): /soc@0/pcie@20000000: simple-bus unit address format error, expected "80000"
>      Rearrange nodes w.r.t. address sort order

NAK, depends on broken PCI bindings being reverted.

Best regards,
Krzysztof

Re: [PATCH v11 0/7] Add PCIe support for Qualcomm IPQ5332

[Krzysztof Wilczyński]

Hello,

> >> Patch series adds support for enabling the PCIe controller and
> >> UNIPHY found on Qualcomm IPQ5332 platform. PCIe0 is Gen3 X1 and
> >> PCIe1 is Gen3 X2 are added.
> > 
> > Applied to dt-bindings, thank you!
> I will send reverts for these. This patchset affects users without
> mentioning it and without providing any rationale.
> 
> What's more, it introduces known to author warnings just to fix them
> later...

The following commit:

  829aa3693f8d ("dt-bindings: PCI: qcom: Use SDX55 'reg' definition for IPQ9574")

Should no longer be present.  However, we still carry the following commit:

  f67d04b18337 ("dt-bindings: PCI: qcom: Document the IPQ5332 PCIe controller")

Let me know if you want it to be removed, too.

Thank you!

	Krzysztof

Re: [PATCH v11 0/7] Add PCIe support for Qualcomm IPQ5332

[Krzysztof Kozlowski]

On 06/03/2025 13:59, Krzysztof Wilczyński wrote:
> Hello,
> 
>>>> Patch series adds support for enabling the PCIe controller and
>>>> UNIPHY found on Qualcomm IPQ5332 platform. PCIe0 is Gen3 X1 and
>>>> PCIe1 is Gen3 X2 are added.
>>>
>>> Applied to dt-bindings, thank you!
>> I will send reverts for these. This patchset affects users without
>> mentioning it and without providing any rationale.
>>
>> What's more, it introduces known to author warnings just to fix them
>> later...
> 
> The following commit:
> 
>   829aa3693f8d ("dt-bindings: PCI: qcom: Use SDX55 'reg' definition for IPQ9574")
> 
> Should no longer be present.  However, we still carry the following commit:
> 
>   f67d04b18337 ("dt-bindings: PCI: qcom: Document the IPQ5332 PCIe controller")


Thanks, I looked briefly and it seemed fine.

Best regards,
Krzysztof

Re: [PATCH v11 3/7] dt-bindings: PCI: qcom: Use sdx55 reg description for ipq9574

[Varadarajan Narayanan]

On Thu, Mar 06, 2025 at 01:06:13PM +0100, Krzysztof Kozlowski wrote:
> On 06/03/2025 12:52, Krzysztof Kozlowski wrote:
> > On 20/02/2025 10:42, Varadarajan Narayanan wrote:
> >> All DT entries except "reg" is similar between ipq5332 and ipq9574. ipq9574
> >> has 5 registers while ipq5332 has 6. MHI is the additional (i.e. sixth
> >> entry). Since this matches with the sdx55's "reg" definition which allows
> >> for 5 or 6 registers, combine ipq9574 with sdx55.
> >>
> >> This change is to prepare ipq9574 to be used as ipq5332's fallback
> >> compatible.
> >>
> >> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >
> > Unreviewed.
> >
> >> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> >> ---
> >> v8: Add 'Reviewed-by: Krzysztof Kozlowski'
> >> ---
> >>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >> index 7235d6554cfb..4b4927178abc 100644
> >> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >> @@ -169,7 +169,6 @@ allOf:
> >>              enum:
> >>                - qcom,pcie-ipq6018
> >>                - qcom,pcie-ipq8074-gen3
> >> -              - qcom,pcie-ipq9574
> >
> > Why you did not explain that you are going to affect users of DTS?
> >
> > NAK

Sorry for not explicitly calling this out. I thought that would be seen from the
following DTS related patches.

> I did not connect the dots, but I pointed out that you break users and
> your DTS is wrong:
> https://lore.kernel.org/all/f7551daa-cce5-47b3-873f-21b9c5026ed2@kernel.org/
>
> so you should come back with questions to clarify what to do, not keep
> pushing this incorrect patchset.
>
> My bad, I should really have zero trust.

It looks like it is not possible to have ipq9574 as fallback (for ipq5332)
without making changes to ipq9574 since the "reg" constraint is different
between the two. And this in turn would break the ABI w.r.t. ipq9574.

To overcome this, two approaches seem to be availabe

	1. Document that ipq9574 is impacted and rework these patches to
	   minimize the impact as much as possible

		(or)

	2. Handle ipq5332 as a separate compatible (without fallback) and reuse
	   the constraints of sdx55 for "reg" and ipq9574 for the others (like
	   clock etc.). This approach will also have to revert [1], as it
	   assumes ipq9574 as fallback.

Please advice which of the above would be appropriate. If there is a better 3rd
alternative please let me know, will align with that approach.

Thanks
Varada

1 - https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/Documentation/devicetree/bindings/pci/qcom,pcie.yaml?id=f67d04b18337249b0faa5cab6223c0bb203f6333

Re: [PATCH v11 4/7] arm64: dts: qcom: ipq9574: Reorder reg and reg-names

[Varadarajan Narayanan]

On Thu, Mar 06, 2025 at 12:56:02PM +0100, Krzysztof Kozlowski wrote:
> On 06/03/2025 12:49, Krzysztof Kozlowski wrote:
> > On 20/02/2025 10:42, Varadarajan Narayanan wrote:
> >> The 'reg' & 'reg-names' constraints used in the bindings and dtsi are
> >> different resulting in dt_bindings_check errors. Re-order the reg entries,
> >
> > Why?

Initially ipq9574 had 5 reg entries. ipq5332 has 6. To be able to use ipq9574 as
fallback for ipq5332 had to add the sixth entry to ipq9574. Then it becomes
similar to sdx55. Hence to avoid duplication, changed ipq9574 to use sdx55 reg
definition. Because of this the erg entries' order changed.

> >
> >> fix the node names and move the nodes to maintain sort order to address the
> >
> > Fixing (how?) node name looks like separate problem.

Because the reg entries order changed, the "parf" register became the first
entry. This resulted in the address in pcie@xxx to not match with the first reg
entry and this was changed. Since the nodes have to be located per address sort
order, had to move the node to an appropriate slot per the address sort order.

> >> following errors/warnings.
> >>
> >> 	arch/arm64/boot/dts/qcom/ipq9574-rdp449.dtb: pcie@20000000: reg-names:0: 'parf' was expected
>
> How can I reproduce this error?
>
> Isn't this error which you intentionally added and now you claim you
> fix? In the same patchset?
>
> This really looks like breaking things just to call it "look, I fixed
> something" two patches later in the same set.

True. But had to do these to have ipq9574 as fallback compatible. Have asked for
suggestions to handle this better. Will follow the approach that is acceptable
to the community.

Thanks
Varada

Re: [PATCH v11 3/7] dt-bindings: PCI: qcom: Use sdx55 reg description for ipq9574

[Krzysztof Kozlowski]

On 10/03/2025 08:44, Varadarajan Narayanan wrote:
> On Thu, Mar 06, 2025 at 01:06:13PM +0100, Krzysztof Kozlowski wrote:
>> On 06/03/2025 12:52, Krzysztof Kozlowski wrote:
>>> On 20/02/2025 10:42, Varadarajan Narayanan wrote:
>>>> All DT entries except "reg" is similar between ipq5332 and ipq9574. ipq9574
>>>> has 5 registers while ipq5332 has 6. MHI is the additional (i.e. sixth
>>>> entry). Since this matches with the sdx55's "reg" definition which allows
>>>> for 5 or 6 registers, combine ipq9574 with sdx55.
>>>>
>>>> This change is to prepare ipq9574 to be used as ipq5332's fallback
>>>> compatible.
>>>>
>>>> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>
>>> Unreviewed.
>>>
>>>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
>>>> ---
>>>> v8: Add 'Reviewed-by: Krzysztof Kozlowski'
>>>> ---
>>>>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>> index 7235d6554cfb..4b4927178abc 100644
>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>> @@ -169,7 +169,6 @@ allOf:
>>>>              enum:
>>>>                - qcom,pcie-ipq6018
>>>>                - qcom,pcie-ipq8074-gen3
>>>> -              - qcom,pcie-ipq9574
>>>
>>> Why you did not explain that you are going to affect users of DTS?
>>>
>>> NAK
> 
> Sorry for not explicitly calling this out. I thought that would be seen from the
> following DTS related patches.
> 
>> I did not connect the dots, but I pointed out that you break users and
>> your DTS is wrong:
>> https://lore.kernel.org/all/f7551daa-cce5-47b3-873f-21b9c5026ed2@kernel.org/
>>
>> so you should come back with questions to clarify what to do, not keep
>> pushing this incorrect patchset.
>>
>> My bad, I should really have zero trust.
> 
> It looks like it is not possible to have ipq9574 as fallback (for ipq5332)
> without making changes to ipq9574 since the "reg" constraint is different
> between the two. And this in turn would break the ABI w.r.t. ipq9574.

I don't get why this is not possible. You have one list for ipq9574 and
existing compatible devices, and you add second list for new device.

... or you just keep existing order. Why you need to keep changing order
every time you add new device?


> 
> To overcome this, two approaches seem to be availabe
> 
> 	1. Document that ipq9574 is impacted and rework these patches to
> 	   minimize the impact as much as possible

What impact? What is the reason to impact ipq9574? What is the actual issue?

> 
> 		(or)
> 
> 	2. Handle ipq5332 as a separate compatible (without fallback) and reuse
> 	   the constraints of sdx55 for "reg" and ipq9574 for the others (like
> 	   clock etc.). This approach will also have to revert [1], as it
> 	   assumes ipq9574 as fallback.
> 
> Please advice which of the above would be appropriate. If there is a better 3rd
> alternative please let me know, will align with that approach.

Keep existing order. Why every time we see new device, it comes up with
a different order?

Best regards,
Krzysztof

Re: [PATCH v11 3/7] dt-bindings: PCI: qcom: Use sdx55 reg description for ipq9574

[Varadarajan Narayanan]

On Mon, Mar 10, 2025 at 12:37:28PM +0100, Krzysztof Kozlowski wrote:
> On 10/03/2025 08:44, Varadarajan Narayanan wrote:
> > On Thu, Mar 06, 2025 at 01:06:13PM +0100, Krzysztof Kozlowski wrote:
> >> On 06/03/2025 12:52, Krzysztof Kozlowski wrote:
> >>> On 20/02/2025 10:42, Varadarajan Narayanan wrote:
> >>>> All DT entries except "reg" is similar between ipq5332 and ipq9574. ipq9574
> >>>> has 5 registers while ipq5332 has 6. MHI is the additional (i.e. sixth
> >>>> entry). Since this matches with the sdx55's "reg" definition which allows
> >>>> for 5 or 6 registers, combine ipq9574 with sdx55.
> >>>>
> >>>> This change is to prepare ipq9574 to be used as ipq5332's fallback
> >>>> compatible.
> >>>>
> >>>> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>>
> >>> Unreviewed.
> >>>
> >>>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> >>>> ---
> >>>> v8: Add 'Reviewed-by: Krzysztof Kozlowski'
> >>>> ---
> >>>>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >>>> index 7235d6554cfb..4b4927178abc 100644
> >>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >>>> @@ -169,7 +169,6 @@ allOf:
> >>>>              enum:
> >>>>                - qcom,pcie-ipq6018
> >>>>                - qcom,pcie-ipq8074-gen3
> >>>> -              - qcom,pcie-ipq9574
> >>>
> >>> Why you did not explain that you are going to affect users of DTS?
> >>>
> >>> NAK
> >
> > Sorry for not explicitly calling this out. I thought that would be seen from the
> > following DTS related patches.
> >
> >> I did not connect the dots, but I pointed out that you break users and
> >> your DTS is wrong:
> >> https://lore.kernel.org/all/f7551daa-cce5-47b3-873f-21b9c5026ed2@kernel.org/
> >>
> >> so you should come back with questions to clarify what to do, not keep
> >> pushing this incorrect patchset.
> >>
> >> My bad, I should really have zero trust.
> >
> > It looks like it is not possible to have ipq9574 as fallback (for ipq5332)
> > without making changes to ipq9574 since the "reg" constraint is different
> > between the two. And this in turn would break the ABI w.r.t. ipq9574.
>
> I don't get why this is not possible. You have one list for ipq9574 and
> existing compatible devices, and you add second list for new device.
>
> ... or you just keep existing order. Why you need to keep changing order
> every time you add new device?

Presently, sdx55 and ipq9574 have the following reg/reg-names constraints.

	compatible	| qcom,pcie-sdx55	| qcom,pcie-ipq9574
	----------------+-----------------------+------------------
        reg	minItems| 5			| 5
		maxItems| 6			| 5
	----------------+-----------------------+------------------
        reg-names	|			|
		minItems| 5			| 5
	----------------+-----------------------+------------------
		maxItems|			| 5 (6 for ipq5332)
	----------------+-----------------------+------------------
		items	|			|
			| parf			| dbi
			| dbi			| elbi
			| elbi			| atu
			| atu			| parf
			| config		| config
			| mhi			| (add mhi for ipq5332)
	----------------+-----------------------+------------------

To make ipq9574 as fallback for ipq5332, have to add "mhi" to reg-names of
ipq9574. Once I add that, the sdx55 and ipq9574 is the same list but in
different order.

If this would not be considered as duplication of the same constraint, then I
can club ipq5332 with ipq9574.

If this would be considered as duplication, then sdx55 and ipq9574 would have to
use the same reg-names list and sdx55 or ipq9574 reg-names order would change.

> > To overcome this, two approaches seem to be availabe
> >
> > 	1. Document that ipq9574 is impacted and rework these patches to
> > 	   minimize the impact as much as possible
>
> What impact? What is the reason to impact ipq9574? What is the actual issue?

By impact, I meant the change in the reg-names order as mentioned above (for
considered as duplication).

> > 		(or)
> >
> > 	2. Handle ipq5332 as a separate compatible (without fallback) and reuse
> > 	   the constraints of sdx55 for "reg" and ipq9574 for the others (like
> > 	   clock etc.). This approach will also have to revert [1], as it
> > 	   assumes ipq9574 as fallback.
> >
> > Please advice which of the above would be appropriate. If there is a better 3rd
> > alternative please let me know, will align with that approach.
>
> Keep existing order. Why every time we see new device, it comes up with
> a different order?

Will be able to do that based on the answer to 'duplication' question and how to
handle that.

	if (adding mhi to ipq9574 reg-names != duplication)

		/* Keep existing order */

		* Append "mhi" to ipq9574
		* use ipq9574 reg-names order for ipq5332

	else
		* combine ipq9574 & sdx55 reg-names

		if (use sdx55 reg-names order)

			/* patchset v11 is using this approach */

			* change ipq9574
			* follow the same for ipq5332

		else if (use ipq9574 order)

			* change sdx55
			* follow the same for ipq5332

Please advice.

Thanks
Varada

Re: [PATCH v11 3/7] dt-bindings: PCI: qcom: Use sdx55 reg description for ipq9574

[Krzysztof Kozlowski]

On 11/03/2025 06:01, Varadarajan Narayanan wrote:
> On Mon, Mar 10, 2025 at 12:37:28PM +0100, Krzysztof Kozlowski wrote:
>> On 10/03/2025 08:44, Varadarajan Narayanan wrote:
>>> On Thu, Mar 06, 2025 at 01:06:13PM +0100, Krzysztof Kozlowski wrote:
>>>> On 06/03/2025 12:52, Krzysztof Kozlowski wrote:
>>>>> On 20/02/2025 10:42, Varadarajan Narayanan wrote:
>>>>>> All DT entries except "reg" is similar between ipq5332 and ipq9574. ipq9574
>>>>>> has 5 registers while ipq5332 has 6. MHI is the additional (i.e. sixth
>>>>>> entry). Since this matches with the sdx55's "reg" definition which allows
>>>>>> for 5 or 6 registers, combine ipq9574 with sdx55.
>>>>>>
>>>>>> This change is to prepare ipq9574 to be used as ipq5332's fallback
>>>>>> compatible.
>>>>>>
>>>>>> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>
>>>>> Unreviewed.
>>>>>
>>>>>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
>>>>>> ---
>>>>>> v8: Add 'Reviewed-by: Krzysztof Kozlowski'
>>>>>> ---
>>>>>>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>>>> index 7235d6554cfb..4b4927178abc 100644
>>>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>>>> @@ -169,7 +169,6 @@ allOf:
>>>>>>              enum:
>>>>>>                - qcom,pcie-ipq6018
>>>>>>                - qcom,pcie-ipq8074-gen3
>>>>>> -              - qcom,pcie-ipq9574
>>>>>
>>>>> Why you did not explain that you are going to affect users of DTS?
>>>>>
>>>>> NAK
>>>
>>> Sorry for not explicitly calling this out. I thought that would be seen from the
>>> following DTS related patches.
>>>
>>>> I did not connect the dots, but I pointed out that you break users and
>>>> your DTS is wrong:
>>>> https://lore.kernel.org/all/f7551daa-cce5-47b3-873f-21b9c5026ed2@kernel.org/
>>>>
>>>> so you should come back with questions to clarify what to do, not keep
>>>> pushing this incorrect patchset.
>>>>
>>>> My bad, I should really have zero trust.
>>>
>>> It looks like it is not possible to have ipq9574 as fallback (for ipq5332)
>>> without making changes to ipq9574 since the "reg" constraint is different
>>> between the two. And this in turn would break the ABI w.r.t. ipq9574.
>>
>> I don't get why this is not possible. You have one list for ipq9574 and
>> existing compatible devices, and you add second list for new device.
>>
>> ... or you just keep existing order. Why you need to keep changing order
>> every time you add new device?
> 
> Presently, sdx55 and ipq9574 have the following reg/reg-names constraints.
> 
> 	compatible	| qcom,pcie-sdx55	| qcom,pcie-ipq9574
> 	----------------+-----------------------+------------------
>         reg	minItems| 5			| 5
> 		maxItems| 6			| 5
> 	----------------+-----------------------+------------------
>         reg-names	|			|
> 		minItems| 5			| 5
> 	----------------+-----------------------+------------------
> 		maxItems|			| 5 (6 for ipq5332)
> 	----------------+-----------------------+------------------
> 		items	|			|
> 			| parf			| dbi
> 			| dbi			| elbi
> 			| elbi			| atu
> 			| atu			| parf
> 			| config		| config
> 			| mhi			| (add mhi for ipq5332)
> 	----------------+-----------------------+------------------
> 
> To make ipq9574 as fallback for ipq5332, have to add "mhi" to reg-names of
> ipq9574. 

only ipq5332 gets additional item, not ipq9574. Your sentence is not
correct. You do not have to add mhi to ipq9574. Neither we, nor schema
asked you to do this.


> Once I add that, the sdx55 and ipq9574 is the same list but in
> different order.
> 

You cannot change the order in existing devices.

> If this would not be considered as duplication of the same constraint, then I
> can club ipq5332 with ipq9574.
> 
> If this would be considered as duplication, then sdx55 and ipq9574 would have to
> use the same reg-names list and sdx55 or ipq9574 reg-names order would change.
> 
>>> To overcome this, two approaches seem to be availabe
>>>
>>> 	1. Document that ipq9574 is impacted and rework these patches to
>>> 	   minimize the impact as much as possible
>>
>> What impact? What is the reason to impact ipq9574? What is the actual issue?
> 
> By impact, I meant the change in the reg-names order as mentioned above (for
> considered as duplication).

Then you must eliminate the impact, not minimize it.

> 
>>> 		(or)
>>>
>>> 	2. Handle ipq5332 as a separate compatible (without fallback) and reuse
>>> 	   the constraints of sdx55 for "reg" and ipq9574 for the others (like
>>> 	   clock etc.). This approach will also have to revert [1], as it
>>> 	   assumes ipq9574 as fallback.
>>>
>>> Please advice which of the above would be appropriate. If there is a better 3rd
>>> alternative please let me know, will align with that approach.
>>
>> Keep existing order. Why every time we see new device, it comes up with
>> a different order?
> 
> Will be able to do that based on the answer to 'duplication' question and how to
> handle that.

I don't understand what is duplication of something here.

> 
> 	if (adding mhi to ipq9574 reg-names != duplication)
> 
> 		/* Keep existing order */
> 
> 		* Append "mhi" to ipq9574

ipq9574 does not have mhi, does it?

If it has, it should be separate patch with its own explanation of the
hardware.


Best regards,
Krzysztof

Re: [PATCH v11 3/7] dt-bindings: PCI: qcom: Use sdx55 reg description for ipq9574

[Varadarajan Narayanan]

On Tue, Mar 11, 2025 at 08:54:55AM +0100, Krzysztof Kozlowski wrote:
> On 11/03/2025 06:01, Varadarajan Narayanan wrote:
> > On Mon, Mar 10, 2025 at 12:37:28PM +0100, Krzysztof Kozlowski wrote:
> >> On 10/03/2025 08:44, Varadarajan Narayanan wrote:
> >>> On Thu, Mar 06, 2025 at 01:06:13PM +0100, Krzysztof Kozlowski wrote:
> >>>> On 06/03/2025 12:52, Krzysztof Kozlowski wrote:
> >>>>> On 20/02/2025 10:42, Varadarajan Narayanan wrote:
> >>>>>> All DT entries except "reg" is similar between ipq5332 and ipq9574. ipq9574
> >>>>>> has 5 registers while ipq5332 has 6. MHI is the additional (i.e. sixth
> >>>>>> entry). Since this matches with the sdx55's "reg" definition which allows
> >>>>>> for 5 or 6 registers, combine ipq9574 with sdx55.
> >>>>>>
> >>>>>> This change is to prepare ipq9574 to be used as ipq5332's fallback
> >>>>>> compatible.
> >>>>>>
> >>>>>> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>>>>
> >>>>> Unreviewed.
> >>>>>
> >>>>>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> >>>>>> ---
> >>>>>> v8: Add 'Reviewed-by: Krzysztof Kozlowski'
> >>>>>> ---
> >>>>>>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
> >>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >>>>>> index 7235d6554cfb..4b4927178abc 100644
> >>>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >>>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >>>>>> @@ -169,7 +169,6 @@ allOf:
> >>>>>>              enum:
> >>>>>>                - qcom,pcie-ipq6018
> >>>>>>                - qcom,pcie-ipq8074-gen3
> >>>>>> -              - qcom,pcie-ipq9574
> >>>>>
> >>>>> Why you did not explain that you are going to affect users of DTS?
> >>>>>
> >>>>> NAK
> >>>
> >>> Sorry for not explicitly calling this out. I thought that would be seen from the
> >>> following DTS related patches.
> >>>
> >>>> I did not connect the dots, but I pointed out that you break users and
> >>>> your DTS is wrong:
> >>>> https://lore.kernel.org/all/f7551daa-cce5-47b3-873f-21b9c5026ed2@kernel.org/
> >>>>
> >>>> so you should come back with questions to clarify what to do, not keep
> >>>> pushing this incorrect patchset.
> >>>>
> >>>> My bad, I should really have zero trust.
> >>>
> >>> It looks like it is not possible to have ipq9574 as fallback (for ipq5332)
> >>> without making changes to ipq9574 since the "reg" constraint is different
> >>> between the two. And this in turn would break the ABI w.r.t. ipq9574.
> >>
> >> I don't get why this is not possible. You have one list for ipq9574 and
> >> existing compatible devices, and you add second list for new device.
> >>
> >> ... or you just keep existing order. Why you need to keep changing order
> >> every time you add new device?
> >
> > Presently, sdx55 and ipq9574 have the following reg/reg-names constraints.
> >
> > 	compatible	| qcom,pcie-sdx55	| qcom,pcie-ipq9574
> > 	----------------+-----------------------+------------------
> >         reg	minItems| 5			| 5
> > 		maxItems| 6			| 5
> > 	----------------+-----------------------+------------------
> >         reg-names	|			|
> > 		minItems| 5			| 5
> > 	----------------+-----------------------+------------------
> > 		maxItems|			| 5 (6 for ipq5332)
> > 	----------------+-----------------------+------------------
> > 		items	|			|
> > 			| parf			| dbi
> > 			| dbi			| elbi
> > 			| elbi			| atu
> > 			| atu			| parf
> > 			| config		| config
> > 			| mhi			| (add mhi for ipq5332)
> > 	----------------+-----------------------+------------------
> >
> > To make ipq9574 as fallback for ipq5332, have to add "mhi" to reg-names of
> > ipq9574.
>
> only ipq5332 gets additional item, not ipq9574. Your sentence is not
> correct. You do not have to add mhi to ipq9574. Neither we, nor schema
> asked you to do this.
>
>
> > Once I add that, the sdx55 and ipq9574 is the same list but in
> > different order.
> >
>
> You cannot change the order in existing devices.

Ok.

> > If this would not be considered as duplication of the same constraint, then I
> > can club ipq5332 with ipq9574.
> >
> > If this would be considered as duplication, then sdx55 and ipq9574 would have to
> > use the same reg-names list and sdx55 or ipq9574 reg-names order would change.
> >
> >>> To overcome this, two approaches seem to be availabe
> >>>
> >>> 	1. Document that ipq9574 is impacted and rework these patches to
> >>> 	   minimize the impact as much as possible
> >>
> >> What impact? What is the reason to impact ipq9574? What is the actual issue?
> >
> > By impact, I meant the change in the reg-names order as mentioned above (for
> > considered as duplication).
>
> Then you must eliminate the impact, not minimize it.

Ok.

> >>> 		(or)
> >>>
> >>> 	2. Handle ipq5332 as a separate compatible (without fallback) and reuse
> >>> 	   the constraints of sdx55 for "reg" and ipq9574 for the others (like
> >>> 	   clock etc.). This approach will also have to revert [1], as it
> >>> 	   assumes ipq9574 as fallback.
> >>>
> >>> Please advice which of the above would be appropriate. If there is a better 3rd
> >>> alternative please let me know, will align with that approach.
> >>
> >> Keep existing order. Why every time we see new device, it comes up with
> >> a different order?
> >
> > Will be able to do that based on the answer to 'duplication' question and how to
> > handle that.
>
> I don't understand what is duplication of something here.

By duplication, I meant the same set of reg-names (in different order).

> > 	if (adding mhi to ipq9574 reg-names != duplication)
> >
> > 		/* Keep existing order */
> >
> > 		* Append "mhi" to ipq9574
>
> ipq9574 does not have mhi, does it?

ipq9574 also has it.

> If it has, it should be separate patch with its own explanation of the
> hardware.

Will discard these patches from the patchset -

	dt-bindings: PCI: qcom: Use sdx55 reg description for ipq9574 Varadarajan Narayanan
	arm64: dts: qcom: ipq9574: Reorder reg and reg-names Varadarajan Narayanan

Will add mhi for ipq9574 and post the next version. Is that ok?

Thanks
Varada

Re: [PATCH v11 3/7] dt-bindings: PCI: qcom: Use sdx55 reg description for ipq9574

[Krzysztof Kozlowski]

On 11/03/2025 09:16, Varadarajan Narayanan wrote:
>> I don't understand what is duplication of something here.
> 
> By duplication, I meant the same set of reg-names (in different order).
> 
>>> 	if (adding mhi to ipq9574 reg-names != duplication)
>>>
>>> 		/* Keep existing order */
>>>
>>> 		* Append "mhi" to ipq9574
>>
>> ipq9574 does not have mhi, does it?
> 
> ipq9574 also has it.

Current binding says no, so something is missing.

> 
>> If it has, it should be separate patch with its own explanation of the
>> hardware.
> 
> Will discard these patches from the patchset -
> 
> 	dt-bindings: PCI: qcom: Use sdx55 reg description for ipq9574 Varadarajan Narayanan
> 	arm64: dts: qcom: ipq9574: Reorder reg and reg-names Varadarajan Narayanan
> 
> Will add mhi for ipq9574 and post the next version. Is that ok?

Yes.

Best regards,
Krzysztof

Re: (subset) [PATCH v11 0/7] Add PCIe support for Qualcomm IPQ5332

[Vinod Koul]

On Thu, 20 Feb 2025 15:12:44 +0530, Varadarajan Narayanan wrote:
> Patch series adds support for enabling the PCIe controller and
> UNIPHY found on Qualcomm IPQ5332 platform. PCIe0 is Gen3 X1 and
> PCIe1 is Gen3 X2 are added.
> 
> This series combines [1] and [2]. [1] introduces IPQ5018 PCIe
> support and [2] depends on [1] to introduce IPQ5332 PCIe support.
> Since the community was interested in [2] (please see [3]), tried
> to revive IPQ5332's PCIe support with v2 of this patch series.
> 
> [...]

Applied, thanks!

[1/7] dt-bindings: phy: qcom,uniphy-pcie: Document PCIe uniphy
      commit: a2e934885c82912caf3f72b9511ef626f3619e3d
[2/7] phy: qcom: Introduce PCIe UNIPHY 28LP driver
      commit: 74badb8b0b146668cc6c03eb58e2a814f9463d02

Best regards,
-- 
~Vinod