devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/2] PCI: qcom: Move PERST# GPIO & phy retrieval from controller to PCIe bridge node
@ 2025-07-02 11:20 Krishna Chaitanya Chundru
  2025-07-02 11:20 ` [PATCH v5 1/2] dt-bindings: PCI: qcom: Move phy & reset gpio's to root port Krishna Chaitanya Chundru
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-07-02 11:20 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	cros-qcom-dts-watchers
  Cc: linux-arm-msm, linux-pci, devicetree, linux-kernel, quic_vbadigan,
	quic_mrana, Krishna Chaitanya Chundru

The main intention of this series is to move wake# to the root port node.
After this series we will come up with a patch which registers for wake IRQ
from the pcieport driver. The wake IRQ is needed for the endpoint to wakeup
the host from D3cold. The driver change for wake IRQ is posted here[1].

There are many places we agreed to move the wake and perst gpio's
and phy etc to the pcie root port node instead of bridge node[2] as the
these properties are root port specific and does not belongs to
bridge node.

So move the phy, phy-names, wake-gpio's in the root port.
There is already reset-gpio defined for PERST# in pci-bus-common.yaml,
start using that property instead of perst-gpio.

For backward compatibility, don't remove any existing properties in the
bridge node.

There are some other properties like num-lanes, max-link-speed which
needs to be moved to the root port nodes, but in this series we are
excluding them for now as this requires more changes in dwc layer and
can complicate the things.

Once this series gets merged all other platforms also will be updated
to use this new way.

[1] https://lore.kernel.org/all/20250401-wake_irq_support-v1-0-d2e22f4a0efd@oss.qualcomm.com/
[2] https://lore.kernel.org/linux-pci/20241211192014.GA3302752@bhelgaas/

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v5:
- Rebased with pci controller/qcom.
- As part of rebase handled one err condition by adding goto err_port_del
- Link to v4: https://lore.kernel.org/r/20250605-perst-v4-0-efe8a0905c27@oss.qualcomm.com

Changes in v4:
- Removed dts patch as Mani suggested to merge driver and dt-binding
  patch in this release and have dts changes in the next release.
- Remove wake property from as this will be addressed in
  pci-bus-common.yaml (Mani)
- Did couple of nits in the comments, function names code etc (Mani).
- Link to v3: https://lore.kernel.org/r/20250419-perst-v3-0-1afec3c4ea62@oss.qualcomm.com

Changes in v3:
- Make old properties as deprecated, update commit message (Dmitry)
- Add helper functions wherever both multiport and legacy methods are used. (Mani)
- Link to v2: https://lore.kernel.org/r/20250414-perst-v2-0-89247746d755@oss.qualcomm.com

Changes in v2:
- Remove phy-names property and change the driver, dtsi accordingly (Rob)
- Link to v1: https://lore.kernel.org/r/20250322-perst-v1-0-e5e4da74a204@oss.qualcomm.com

---
Krishna Chaitanya Chundru (2):
      dt-bindings: PCI: qcom: Move phy & reset gpio's to root port
      PCI: qcom: Add support for multi-root port

 .../devicetree/bindings/pci/qcom,pcie-common.yaml  |  32 +++-
 .../devicetree/bindings/pci/qcom,pcie-sc7280.yaml  |  16 +-
 drivers/pci/controller/dwc/pcie-qcom.c             | 178 +++++++++++++++++----
 3 files changed, 193 insertions(+), 33 deletions(-)
---
base-commit: 7c184aa42a3dc9b2630010fbcb06c701c440f8e3
change-id: 20250702-perst-973129cfc9da

Best regards,
-- 
krishnachaitanya-linux <krichai@qti.qualcomm.com>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v5 1/2] dt-bindings: PCI: qcom: Move phy & reset gpio's to root port
  2025-07-02 11:20 [PATCH v5 0/2] PCI: qcom: Move PERST# GPIO & phy retrieval from controller to PCIe bridge node Krishna Chaitanya Chundru
@ 2025-07-02 11:20 ` Krishna Chaitanya Chundru
  2025-07-02 11:20 ` [PATCH v5 2/2] PCI: qcom: Add support for multi-root port Krishna Chaitanya Chundru
  2025-07-03  8:30 ` [PATCH v5 0/2] PCI: qcom: Move PERST# GPIO & phy retrieval from controller to PCIe bridge node Manivannan Sadhasivam
  2 siblings, 0 replies; 7+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-07-02 11:20 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	cros-qcom-dts-watchers
  Cc: linux-arm-msm, linux-pci, devicetree, linux-kernel, quic_vbadigan,
	quic_mrana, Krishna Chaitanya Chundru

Move the phys, phy-names etc to the pcie root port node from host bridge
node, as agreed upon in multiple places one instance is[1].

Update the qcom,pcie-common.yaml to include the phys, phy-names properties
in the root port node. There is already reset-gpios defined for PERST# in
pci-bus-common.yaml, start using that property instead of perst-gpio.

For backward compatibility, do not remove any existing properties in the
bridge node. Hence mark them as 'deprecated'.

[1] https://lore.kernel.org/linux-pci/20241211192014.GA3302752@bhelgaas/

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../devicetree/bindings/pci/qcom,pcie-common.yaml  | 32 ++++++++++++++++++++--
 .../devicetree/bindings/pci/qcom,pcie-sc7280.yaml  | 16 ++++++++---
 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
index 0480c58f7d998adbac4c6de20cdaec945b3bab21..ab2509ec1c4b40ac91a93033d1bab1b12c39362f 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
@@ -51,10 +51,18 @@ properties:
 
   phys:
     maxItems: 1
+    deprecated: true
+    description:
+      This property is deprecated, instead of referencing this property from
+      the host bridge node, use the property from the PCIe root port node.
 
   phy-names:
     items:
       - const: pciephy
+    deprecated: true
+    description:
+      Phandle to the register map node. This property is deprecated, and not
+      required to add in the root port also, as the root port has only one phy.
 
   power-domains:
     maxItems: 1
@@ -71,12 +79,18 @@ properties:
     maxItems: 12
 
   perst-gpios:
-    description: GPIO controlled connection to PERST# signal
+    description: GPIO controlled connection to PERST# signal. This property is
+      deprecated, instead of referencing this property from the host bridge node,
+      use the reset-gpios property from the root port node.
     maxItems: 1
+    deprecated: true
 
   wake-gpios:
-    description: GPIO controlled connection to WAKE# signal
+    description: GPIO controlled connection to WAKE# signal. This property is
+      deprecated, instead of referencing this property from the host bridge node,
+      use the property from the PCIe root port node.
     maxItems: 1
+    deprecated: true
 
   vddpe-3v3-supply:
     description: PCIe endpoint power supply
@@ -85,6 +99,20 @@ properties:
   opp-table:
     type: object
 
+patternProperties:
+  "^pcie@":
+    type: object
+    $ref: /schemas/pci/pci-pci-bridge.yaml#
+
+    properties:
+      reg:
+        maxItems: 1
+
+      phys:
+        maxItems: 1
+
+    unevaluatedProperties: false
+
 required:
   - reg
   - reg-names
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
index ff508f592a1acf7557ed8035d819207dab01f94d..4d0a915566030f8fbd8bf83a9ccca00fbc7574bd 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.yaml
@@ -165,9 +165,6 @@ examples:
             iommu-map = <0x0 &apps_smmu 0x1c80 0x1>,
                         <0x100 &apps_smmu 0x1c81 0x1>;
 
-            phys = <&pcie1_phy>;
-            phy-names = "pciephy";
-
             pinctrl-names = "default";
             pinctrl-0 = <&pcie1_clkreq_n>;
 
@@ -176,7 +173,18 @@ examples:
             resets = <&gcc GCC_PCIE_1_BCR>;
             reset-names = "pci";
 
-            perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
             vddpe-3v3-supply = <&pp3300_ssd>;
+            pcie1_port0: pcie@0 {
+                device_type = "pci";
+                reg = <0x0 0x0 0x0 0x0 0x0>;
+                bus-range = <0x01 0xff>;
+
+                #address-cells = <3>;
+                #size-cells = <2>;
+                ranges;
+                phys = <&pcie1_phy>;
+
+                reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
+            };
         };
     };

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 2/2] PCI: qcom: Add support for multi-root port
  2025-07-02 11:20 [PATCH v5 0/2] PCI: qcom: Move PERST# GPIO & phy retrieval from controller to PCIe bridge node Krishna Chaitanya Chundru
  2025-07-02 11:20 ` [PATCH v5 1/2] dt-bindings: PCI: qcom: Move phy & reset gpio's to root port Krishna Chaitanya Chundru
@ 2025-07-02 11:20 ` Krishna Chaitanya Chundru
  2025-07-02 15:33   ` Manivannan Sadhasivam
  2025-07-03  8:30 ` [PATCH v5 0/2] PCI: qcom: Move PERST# GPIO & phy retrieval from controller to PCIe bridge node Manivannan Sadhasivam
  2 siblings, 1 reply; 7+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-07-02 11:20 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	cros-qcom-dts-watchers
  Cc: linux-arm-msm, linux-pci, devicetree, linux-kernel, quic_vbadigan,
	quic_mrana, Krishna Chaitanya Chundru

Move phy, PERST# handling to root port and provide a way to have multi-port
logic.

Currently, QCOM controllers only support single port, and all properties
are present in the host bridge node itself. This is incorrect, as
properties like phys, perst-gpios, etc.. can vary per port and should be
present in the root port node.

To maintain DT backwards compatibility, fallback to the legacy method of
parsing the host bridge node if the port parsing fails.

pci-bus-common.yaml uses reset-gpios property for representing PERST#, use
same property instead of perst-gpios.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 178 ++++++++++++++++++++++++++++-----
 1 file changed, 151 insertions(+), 27 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index f7ed1e010eb6607b2e98a42f0051c47e4de2af93..56d04a15edf8f99f6d3b9bfaa037ff922b521888 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -267,6 +267,12 @@ struct qcom_pcie_cfg {
 	bool no_l0s;
 };
 
+struct qcom_pcie_port {
+	struct list_head list;
+	struct gpio_desc *reset;
+	struct phy *phy;
+};
+
 struct qcom_pcie {
 	struct dw_pcie *pci;
 	void __iomem *parf;			/* DT parf */
@@ -279,24 +285,37 @@ struct qcom_pcie {
 	struct icc_path *icc_cpu;
 	const struct qcom_pcie_cfg *cfg;
 	struct dentry *debugfs;
+	struct list_head ports;
 	bool suspended;
 	bool use_pm_opp;
 };
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
 
-static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
+static void qcom_perst_assert(struct qcom_pcie *pcie, bool assert)
 {
-	gpiod_set_value_cansleep(pcie->reset, 1);
+	struct qcom_pcie_port *port;
+	int val = assert ? 1 : 0;
+
+	if (list_empty(&pcie->ports))
+		gpiod_set_value_cansleep(pcie->reset, val);
+	else
+		list_for_each_entry(port, &pcie->ports, list)
+			gpiod_set_value_cansleep(port->reset, val);
+
 	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
 }
 
+static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
+{
+	qcom_perst_assert(pcie, true);
+}
+
 static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
 {
 	/* Ensure that PERST has been asserted for at least 100 ms */
 	msleep(PCIE_T_PVPERL_MS);
-	gpiod_set_value_cansleep(pcie->reset, 0);
-	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
+	qcom_perst_assert(pcie, false);
 }
 
 static int qcom_pcie_start_link(struct dw_pcie *pci)
@@ -1234,6 +1253,59 @@ static bool qcom_pcie_link_up(struct dw_pcie *pci)
 	return val & PCI_EXP_LNKSTA_DLLLA;
 }
 
+static void qcom_pcie_phy_exit(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_port *port;
+
+	if (list_empty(&pcie->ports))
+		phy_exit(pcie->phy);
+	else
+		list_for_each_entry(port, &pcie->ports, list)
+			phy_exit(port->phy);
+}
+
+static void qcom_pcie_phy_power_off(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_port *port;
+
+	if (list_empty(&pcie->ports)) {
+		phy_power_off(pcie->phy);
+	} else {
+		list_for_each_entry(port, &pcie->ports, list)
+			phy_power_off(port->phy);
+	}
+}
+
+static int qcom_pcie_phy_power_on(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_port *port;
+	int ret = 0;
+
+	if (list_empty(&pcie->ports)) {
+		ret = phy_set_mode_ext(pcie->phy, PHY_MODE_PCIE, PHY_MODE_PCIE_RC);
+		if (ret)
+			return ret;
+
+		ret = phy_power_on(pcie->phy);
+		if (ret)
+			return ret;
+	} else {
+		list_for_each_entry(port, &pcie->ports, list) {
+			ret = phy_set_mode_ext(port->phy, PHY_MODE_PCIE, PHY_MODE_PCIE_RC);
+			if (ret)
+				return ret;
+
+			ret = phy_power_on(port->phy);
+			if (ret) {
+				qcom_pcie_phy_power_off(pcie);
+				return ret;
+			}
+		}
+	}
+
+	return ret;
+}
+
 static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -1246,11 +1318,7 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 	if (ret)
 		return ret;
 
-	ret = phy_set_mode_ext(pcie->phy, PHY_MODE_PCIE, PHY_MODE_PCIE_RC);
-	if (ret)
-		goto err_deinit;
-
-	ret = phy_power_on(pcie->phy);
+	ret = qcom_pcie_phy_power_on(pcie);
 	if (ret)
 		goto err_deinit;
 
@@ -1273,7 +1341,7 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 err_assert_reset:
 	qcom_ep_reset_assert(pcie);
 err_disable_phy:
-	phy_power_off(pcie->phy);
+	qcom_pcie_phy_power_off(pcie);
 err_deinit:
 	pcie->cfg->ops->deinit(pcie);
 
@@ -1286,7 +1354,7 @@ static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
 	struct qcom_pcie *pcie = to_qcom_pcie(pci);
 
 	qcom_ep_reset_assert(pcie);
-	phy_power_off(pcie->phy);
+	qcom_pcie_phy_power_off(pcie);
 	pcie->cfg->ops->deinit(pcie);
 }
 
@@ -1631,11 +1699,41 @@ static const struct pci_ecam_ops pci_qcom_ecam_ops = {
 	}
 };
 
+static int qcom_pcie_parse_port(struct qcom_pcie *pcie, struct device_node *node)
+{
+	struct device *dev = pcie->pci->dev;
+	struct qcom_pcie_port *port;
+	struct gpio_desc *reset;
+	struct phy *phy;
+
+	reset = devm_fwnode_gpiod_get(dev, of_fwnode_handle(node),
+				      "reset", GPIOD_OUT_HIGH, "PERST#");
+	if (IS_ERR(reset))
+		return PTR_ERR(reset);
+
+	phy = devm_of_phy_get(dev, node, NULL);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+	if (!port)
+		return -ENOMEM;
+
+	port->reset = reset;
+	port->phy = phy;
+	INIT_LIST_HEAD(&port->list);
+	list_add_tail(&port->list, &pcie->ports);
+
+	return 0;
+}
+
 static int qcom_pcie_probe(struct platform_device *pdev)
 {
 	const struct qcom_pcie_cfg *pcie_cfg;
 	unsigned long max_freq = ULONG_MAX;
+	struct qcom_pcie_port *port, *tmp;
 	struct device *dev = &pdev->dev;
+	struct device_node *of_port;
 	struct dev_pm_opp *opp;
 	struct qcom_pcie *pcie;
 	struct dw_pcie_rp *pp;
@@ -1701,6 +1799,8 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 		goto err_pm_runtime_put;
 	}
 
+	INIT_LIST_HEAD(&pcie->ports);
+
 	pci->dev = dev;
 	pci->ops = &dw_pcie_ops;
 	pp = &pci->pp;
@@ -1709,12 +1809,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 
 	pcie->cfg = pcie_cfg;
 
-	pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
-	if (IS_ERR(pcie->reset)) {
-		ret = PTR_ERR(pcie->reset);
-		goto err_pm_runtime_put;
-	}
-
 	pcie->parf = devm_platform_ioremap_resource_byname(pdev, "parf");
 	if (IS_ERR(pcie->parf)) {
 		ret = PTR_ERR(pcie->parf);
@@ -1737,12 +1831,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 		}
 	}
 
-	pcie->phy = devm_phy_optional_get(dev, "pciephy");
-	if (IS_ERR(pcie->phy)) {
-		ret = PTR_ERR(pcie->phy);
-		goto err_pm_runtime_put;
-	}
-
 	/* OPP table is optional */
 	ret = devm_pm_opp_of_add_table(dev);
 	if (ret && ret != -ENODEV) {
@@ -1789,9 +1877,42 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 
 	pp->ops = &qcom_pcie_dw_ops;
 
-	ret = phy_init(pcie->phy);
-	if (ret)
-		goto err_pm_runtime_put;
+	for_each_available_child_of_node(dev->of_node, of_port) {
+		ret = qcom_pcie_parse_port(pcie, of_port);
+		of_node_put(of_port);
+		if (ret) {
+			if (ret != -ENOENT) {
+				dev_err_probe(pci->dev, ret,
+					      "Failed to parse port nodes %d\n",
+					      ret);
+				goto err_port_del;
+			}
+			break;
+		}
+	}
+
+	/*
+	 * In the case of properties not populated in root port, fallback to the
+	 * legacy method of parsing the host bridge node. This is to maintain DT
+	 * backwards compatibility.
+	 */
+	if (ret) {
+		pcie->phy = devm_phy_optional_get(dev, "pciephy");
+		if (IS_ERR(pcie->phy)) {
+			ret = PTR_ERR(pcie->phy);
+			goto err_pm_runtime_put;
+		}
+
+		pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
+		if (IS_ERR(pcie->reset)) {
+			ret = PTR_ERR(pcie->reset);
+			goto err_pm_runtime_put;
+		}
+
+		ret = phy_init(pcie->phy);
+		if (ret)
+			goto err_pm_runtime_put;
+	}
 
 	platform_set_drvdata(pdev, pcie);
 
@@ -1836,7 +1957,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 err_host_deinit:
 	dw_pcie_host_deinit(pp);
 err_phy_exit:
-	phy_exit(pcie->phy);
+	qcom_pcie_phy_exit(pcie);
+err_port_del:
+	list_for_each_entry_safe(port, tmp, &pcie->ports, list)
+		list_del(&port->list);
 err_pm_runtime_put:
 	pm_runtime_put(dev);
 	pm_runtime_disable(dev);

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 2/2] PCI: qcom: Add support for multi-root port
  2025-07-02 11:20 ` [PATCH v5 2/2] PCI: qcom: Add support for multi-root port Krishna Chaitanya Chundru
@ 2025-07-02 15:33   ` Manivannan Sadhasivam
  2025-07-03  4:18     ` Krishna Chaitanya Chundru
  0 siblings, 1 reply; 7+ messages in thread
From: Manivannan Sadhasivam @ 2025-07-02 15:33 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
	Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Konrad Dybcio, cros-qcom-dts-watchers, linux-arm-msm, linux-pci,
	devicetree, linux-kernel, quic_vbadigan, quic_mrana

On Wed, Jul 02, 2025 at 04:50:42PM GMT, Krishna Chaitanya Chundru wrote:

[...]

> -	ret = phy_init(pcie->phy);
> -	if (ret)
> -		goto err_pm_runtime_put;
> +	for_each_available_child_of_node(dev->of_node, of_port) {
> +		ret = qcom_pcie_parse_port(pcie, of_port);
> +		of_node_put(of_port);
> +		if (ret) {
> +			if (ret != -ENOENT) {
> +				dev_err_probe(pci->dev, ret,
> +					      "Failed to parse port nodes %d\n",
> +					      ret);
> +				goto err_port_del;
> +			}
> +			break;
> +		}
> +	}
> +
> +	/*
> +	 * In the case of properties not populated in root port, fallback to the
> +	 * legacy method of parsing the host bridge node. This is to maintain DT
> +	 * backwards compatibility.
> +	 */
> +	if (ret) {
> +		pcie->phy = devm_phy_optional_get(dev, "pciephy");
> +		if (IS_ERR(pcie->phy)) {
> +			ret = PTR_ERR(pcie->phy);
> +			goto err_pm_runtime_put;

Shouldn't this and below be err_port_del?

- Mani

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 2/2] PCI: qcom: Add support for multi-root port
  2025-07-02 15:33   ` Manivannan Sadhasivam
@ 2025-07-03  4:18     ` Krishna Chaitanya Chundru
  2025-07-03  6:59       ` Manivannan Sadhasivam
  0 siblings, 1 reply; 7+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-07-03  4:18 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
	Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Konrad Dybcio, cros-qcom-dts-watchers, linux-arm-msm, linux-pci,
	devicetree, linux-kernel, quic_vbadigan, quic_mrana



On 7/2/2025 9:03 PM, Manivannan Sadhasivam wrote:
> On Wed, Jul 02, 2025 at 04:50:42PM GMT, Krishna Chaitanya Chundru wrote:
> 
> [...]
> 
>> -	ret = phy_init(pcie->phy);
>> -	if (ret)
>> -		goto err_pm_runtime_put;
>> +	for_each_available_child_of_node(dev->of_node, of_port) {
>> +		ret = qcom_pcie_parse_port(pcie, of_port);
>> +		of_node_put(of_port);
>> +		if (ret) {
>> +			if (ret != -ENOENT) {
>> +				dev_err_probe(pci->dev, ret,
>> +					      "Failed to parse port nodes %d\n",
>> +					      ret);
>> +				goto err_port_del;
>> +			}
>> +			break;
>> +		}
>> +	}
>> +
>> +	/*
>> +	 * In the case of properties not populated in root port, fallback to the
>> +	 * legacy method of parsing the host bridge node. This is to maintain DT
>> +	 * backwards compatibility.
>> +	 */
>> +	if (ret) {
>> +		pcie->phy = devm_phy_optional_get(dev, "pciephy");
>> +		if (IS_ERR(pcie->phy)) {
>> +			ret = PTR_ERR(pcie->phy);
>> +			goto err_pm_runtime_put;
> 
> Shouldn't this and below be err_port_del?
> 
This is a legacy way of parsing property, if the execution
comes here means the port parsing has failed and ports are not created.
so err_port_del will not have any effect.

- Krishna Chaitanya.
> - Mani
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 2/2] PCI: qcom: Add support for multi-root port
  2025-07-03  4:18     ` Krishna Chaitanya Chundru
@ 2025-07-03  6:59       ` Manivannan Sadhasivam
  0 siblings, 0 replies; 7+ messages in thread
From: Manivannan Sadhasivam @ 2025-07-03  6:59 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
	Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Konrad Dybcio, cros-qcom-dts-watchers, linux-arm-msm, linux-pci,
	devicetree, linux-kernel, quic_vbadigan, quic_mrana

On Thu, Jul 03, 2025 at 09:48:17AM GMT, Krishna Chaitanya Chundru wrote:
> 
> 
> On 7/2/2025 9:03 PM, Manivannan Sadhasivam wrote:
> > On Wed, Jul 02, 2025 at 04:50:42PM GMT, Krishna Chaitanya Chundru wrote:
> > 
> > [...]
> > 
> > > -	ret = phy_init(pcie->phy);
> > > -	if (ret)
> > > -		goto err_pm_runtime_put;
> > > +	for_each_available_child_of_node(dev->of_node, of_port) {
> > > +		ret = qcom_pcie_parse_port(pcie, of_port);
> > > +		of_node_put(of_port);
> > > +		if (ret) {
> > > +			if (ret != -ENOENT) {
> > > +				dev_err_probe(pci->dev, ret,
> > > +					      "Failed to parse port nodes %d\n",
> > > +					      ret);
> > > +				goto err_port_del;
> > > +			}
> > > +			break;
> > > +		}
> > > +	}
> > > +
> > > +	/*
> > > +	 * In the case of properties not populated in root port, fallback to the
> > > +	 * legacy method of parsing the host bridge node. This is to maintain DT
> > > +	 * backwards compatibility.
> > > +	 */
> > > +	if (ret) {
> > > +		pcie->phy = devm_phy_optional_get(dev, "pciephy");
> > > +		if (IS_ERR(pcie->phy)) {
> > > +			ret = PTR_ERR(pcie->phy);
> > > +			goto err_pm_runtime_put;
> > 
> > Shouldn't this and below be err_port_del?
> > 
> This is a legacy way of parsing property, if the execution
> comes here means the port parsing has failed and ports are not created.
> so err_port_del will not have any effect.
> 

Oops. I got confused by the if (ret) flow. It would be more clear if a goto is
used to indicate that the legacy codeblock is skipped. I'll just incorporate it
while applying.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 0/2] PCI: qcom: Move PERST# GPIO & phy retrieval from controller to PCIe bridge node
  2025-07-02 11:20 [PATCH v5 0/2] PCI: qcom: Move PERST# GPIO & phy retrieval from controller to PCIe bridge node Krishna Chaitanya Chundru
  2025-07-02 11:20 ` [PATCH v5 1/2] dt-bindings: PCI: qcom: Move phy & reset gpio's to root port Krishna Chaitanya Chundru
  2025-07-02 11:20 ` [PATCH v5 2/2] PCI: qcom: Add support for multi-root port Krishna Chaitanya Chundru
@ 2025-07-03  8:30 ` Manivannan Sadhasivam
  2 siblings, 0 replies; 7+ messages in thread
From: Manivannan Sadhasivam @ 2025-07-03  8:30 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
	Bjorn Helgaas, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Konrad Dybcio, cros-qcom-dts-watchers, Krishna Chaitanya Chundru
  Cc: Manivannan Sadhasivam, linux-arm-msm, linux-pci, devicetree,
	linux-kernel, quic_vbadigan, quic_mrana


On Wed, 02 Jul 2025 16:50:40 +0530, Krishna Chaitanya Chundru wrote:
> The main intention of this series is to move wake# to the root port node.
> After this series we will come up with a patch which registers for wake IRQ
> from the pcieport driver. The wake IRQ is needed for the endpoint to wakeup
> the host from D3cold. The driver change for wake IRQ is posted here[1].
> 
> There are many places we agreed to move the wake and perst gpio's
> and phy etc to the pcie root port node instead of bridge node[2] as the
> these properties are root port specific and does not belongs to
> bridge node.
> 
> [...]

Applied, thanks!

[1/2] dt-bindings: PCI: qcom: Move phy & reset gpio's to root port
      commit: 77df6234668afa9c44490130e62d5c30734d6e2d
[2/2] PCI: qcom: Add support for multi-root port
      commit: 9e09ecef33e6f1019be921732cc2d7bd945202d8

NOTE: I've made changes to the parsing logic. So please test the new version to
make sure I didn't break anything.

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-07-03  8:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-02 11:20 [PATCH v5 0/2] PCI: qcom: Move PERST# GPIO & phy retrieval from controller to PCIe bridge node Krishna Chaitanya Chundru
2025-07-02 11:20 ` [PATCH v5 1/2] dt-bindings: PCI: qcom: Move phy & reset gpio's to root port Krishna Chaitanya Chundru
2025-07-02 11:20 ` [PATCH v5 2/2] PCI: qcom: Add support for multi-root port Krishna Chaitanya Chundru
2025-07-02 15:33   ` Manivannan Sadhasivam
2025-07-03  4:18     ` Krishna Chaitanya Chundru
2025-07-03  6:59       ` Manivannan Sadhasivam
2025-07-03  8:30 ` [PATCH v5 0/2] PCI: qcom: Move PERST# GPIO & phy retrieval from controller to PCIe bridge node Manivannan Sadhasivam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).