[PATCH] PCI: qcom: Handle mixed PERST#/PHY DT configuration

[PATCH] PCI: qcom: Handle mixed PERST#/PHY DT configuration

[Qiang Yu]

The driver currently supports two PERST# and PHY DT configuration. In one
case, PHY, PERST#, are described in the RC node. In the other case, they
are described in the RP node.

A mixed setup is not supported. One common example is PHY on the RP node
while PERST# remains on the RC node. In that case the driver goes through
the RP parse path, does not find PERST# on RP, and does not report an
error because PERST# is optional. Probe can then succeed silently while
PERST# is left uncontrolled, and PCIe endpoints fails to work later. This
silent probe success makes debugging difficult.

Handle this mixed case in the RP parse path by checking whether PERST# is
present on RC and, if so, using the RC PERST# GPIO for RP ports while
keeping RP parsing for PHY. Emit a warning to indicate mixed DT content so
it can be fixed.

This keeps mixed systems functional and makes the configuration issue
visible instead of failing later at endpoint bring-up.

Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 9fdfc88ac151..8235961d692f 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -282,6 +282,7 @@ struct qcom_pcie {
 	const struct qcom_pcie_cfg *cfg;
 	struct dentry *debugfs;
 	struct list_head ports;
+	struct gpio_desc *reset;
 	bool suspended;
 	bool use_pm_opp;
 };
@@ -1703,6 +1704,11 @@ static int qcom_pcie_parse_perst(struct qcom_pcie *pcie,
 	struct gpio_desc *reset;
 	int ret;
 
+	if (pcie->reset) {
+		reset = pcie->reset;
+		goto skip_perst_parsing;
+	}
+
 	if (!of_find_property(np, "reset-gpios", NULL))
 		goto parse_child_node;
 
@@ -1721,6 +1727,7 @@ static int qcom_pcie_parse_perst(struct qcom_pcie *pcie,
 		return PTR_ERR(reset);
 	}
 
+skip_perst_parsing:
 	perst = devm_kzalloc(dev, sizeof(*perst), GFP_KERNEL);
 	if (!perst)
 		return -ENOMEM;
@@ -1778,6 +1785,14 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
 	struct device *dev = pcie->pci->dev;
 	int ret = -ENODEV;
 
+	if (of_find_property(dev->of_node, "perst-gpios", NULL)) {
+		pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
+		if (IS_ERR(pcie->reset))
+			return PTR_ERR(pcie->reset);
+
+		dev_warn(dev, "Reusing PERST# from Root Complex node. DT needs to be fixed!\n");
+	}
+
 	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
 		if (!of_node_is_type(of_port, "pci"))
 			continue;

---
base-commit: 33a76fc3c3e61386524479b99f35423bd3d9a895
change-id: 20260508-mix_perst_phy_dts-b741b840833b

Best regards,
--  
Qiang Yu <qiang.yu@oss.qualcomm.com>

Re: [PATCH] PCI: qcom: Handle mixed PERST#/PHY DT configuration

[Konrad Dybcio]

On 5/8/26 11:54 AM, Qiang Yu wrote:
> The driver currently supports two PERST# and PHY DT configuration. In one
> case, PHY, PERST#, are described in the RC node. In the other case, they
> are described in the RP node.
> 
> A mixed setup is not supported. One common example is PHY on the RP node
> while PERST# remains on the RC node. In that case the driver goes through
> the RP parse path, does not find PERST# on RP, and does not report an
> error because PERST# is optional. Probe can then succeed silently while
> PERST# is left uncontrolled, and PCIe endpoints fails to work later. This
> silent probe success makes debugging difficult.
> 
> Handle this mixed case in the RP parse path by checking whether PERST# is
> present on RC and, if so, using the RC PERST# GPIO for RP ports while
> keeping RP parsing for PHY. Emit a warning to indicate mixed DT content so
> it can be fixed.
> 
> This keeps mixed systems functional and makes the configuration issue
> visible instead of failing later at endpoint bring-up.
> 
> Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 9fdfc88ac151..8235961d692f 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -282,6 +282,7 @@ struct qcom_pcie {
>  	const struct qcom_pcie_cfg *cfg;
>  	struct dentry *debugfs;
>  	struct list_head ports;
> +	struct gpio_desc *reset;

I'd rename this to make it more obvious it's at the RC node.. but
rc_reset doesn't really sound great..

otherwise:

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH] PCI: qcom: Handle mixed PERST#/PHY DT configuration

[Manivannan Sadhasivam]

On Fri, 08 May 2026 02:54:19 -0700, Qiang Yu wrote:
> The driver currently supports two PERST# and PHY DT configuration. In one
> case, PHY, PERST#, are described in the RC node. In the other case, they
> are described in the RP node.
> 
> A mixed setup is not supported. One common example is PHY on the RP node
> while PERST# remains on the RC node. In that case the driver goes through
> the RP parse path, does not find PERST# on RP, and does not report an
> error because PERST# is optional. Probe can then succeed silently while
> PERST# is left uncontrolled, and PCIe endpoints fails to work later. This
> silent probe success makes debugging difficult.
> 
> [...]

Applied, thanks!

[1/1] PCI: qcom: Handle mixed PERST#/PHY DT configuration
      commit: d160e4d32a8b94e534f7c491374b99cf44baa4e0

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

Re: [PATCH] PCI: qcom: Handle mixed PERST#/PHY DT configuration

[Dmitry Baryshkov]

On Fri, May 08, 2026 at 02:54:19AM -0700, Qiang Yu wrote:
> The driver currently supports two PERST# and PHY DT configuration. In one
> case, PHY, PERST#, are described in the RC node. In the other case, they
> are described in the RP node.
> 
> A mixed setup is not supported. One common example is PHY on the RP node
> while PERST# remains on the RC node. In that case the driver goes through
> the RP parse path, does not find PERST# on RP, and does not report an
> error because PERST# is optional. Probe can then succeed silently while
> PERST# is left uncontrolled, and PCIe endpoints fails to work later. This
> silent probe success makes debugging difficult.
> 
> Handle this mixed case in the RP parse path by checking whether PERST# is
> present on RC and, if so, using the RC PERST# GPIO for RP ports while
> keeping RP parsing for PHY. Emit a warning to indicate mixed DT content so
> it can be fixed.
> 
> This keeps mixed systems functional and makes the configuration issue
> visible instead of failing later at endpoint bring-up.
> 
> Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> @@ -1778,6 +1785,14 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
>  	struct device *dev = pcie->pci->dev;
>  	int ret = -ENODEV;
>  
> +	if (of_find_property(dev->of_node, "perst-gpios", NULL)) {
> +		pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
> +		if (IS_ERR(pcie->reset))
> +			return PTR_ERR(pcie->reset);
> +
> +		dev_warn(dev, "Reusing PERST# from Root Complex node. DT needs to be fixed!\n");

This patch breaks legacy DTs (I stumbled on it on SM8350 HDK), because
now devm_gpiod_get_optional() in qcom_pcie_parse_legacy_binding()
returns -EBUSY (as the GPIO is already requested here). I'll send a
patch fixing the issue, but it's sad to see existing platforms being
broken for the sake of undeclared "mixed" case.


-- 
With best wishes
Dmitry

Re: [PATCH] PCI: qcom: Handle mixed PERST#/PHY DT configuration

[Kathiravan Thirumoorthy]

On 5/31/2026 4:37 PM, Dmitry Baryshkov wrote:
> On Fri, May 08, 2026 at 02:54:19AM -0700, Qiang Yu wrote:
>> The driver currently supports two PERST# and PHY DT configuration. In one
>> case, PHY, PERST#, are described in the RC node. In the other case, they
>> are described in the RP node.
>>
>> A mixed setup is not supported. One common example is PHY on the RP node
>> while PERST# remains on the RC node. In that case the driver goes through
>> the RP parse path, does not find PERST# on RP, and does not report an
>> error because PERST# is optional. Probe can then succeed silently while
>> PERST# is left uncontrolled, and PCIe endpoints fails to work later. This
>> silent probe success makes debugging difficult.
>>
>> Handle this mixed case in the RP parse path by checking whether PERST# is
>> present on RC and, if so, using the RC PERST# GPIO for RP ports while
>> keeping RP parsing for PHY. Emit a warning to indicate mixed DT content so
>> it can be fixed.
>>
>> This keeps mixed systems functional and makes the configuration issue
>> visible instead of failing later at endpoint bring-up.
>>
>> Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> @@ -1778,6 +1785,14 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
>>   	struct device *dev = pcie->pci->dev;
>>   	int ret = -ENODEV;
>>   
>> +	if (of_find_property(dev->of_node, "perst-gpios", NULL)) {
>> +		pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
>> +		if (IS_ERR(pcie->reset))
>> +			return PTR_ERR(pcie->reset);
>> +
>> +		dev_warn(dev, "Reusing PERST# from Root Complex node. DT needs to be fixed!\n");
> This patch breaks legacy DTs (I stumbled on it on SM8350 HDK), because
> now devm_gpiod_get_optional() in qcom_pcie_parse_legacy_binding()
> returns -EBUSY (as the GPIO is already requested here). I'll send a
> patch fixing the issue, but it's sad to see existing platforms being
> broken for the sake of undeclared "mixed" case.

Dmitry,

I have sent[1] fix for this issue and it is squashed with the offending 
commit itself[2] and part of controller/dwc-qcom in the PCI tree.

[1] 
https://lore.kernel.org/linux-arm-msm/20260526-fix_perst_gpio_handling-v1-1-9170507bb4e9@oss.qualcomm.com/T/#u

[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=controller/dwc-qcom&id=1a23bcb452d95f099e530414504c0d99ee076b3f

Re: [PATCH] PCI: qcom: Handle mixed PERST#/PHY DT configuration

[Dmitry Baryshkov]

On Tue, Jun 02, 2026 at 10:04:22AM +0530, Kathiravan Thirumoorthy wrote:
> 
> On 5/31/2026 4:37 PM, Dmitry Baryshkov wrote:
> > On Fri, May 08, 2026 at 02:54:19AM -0700, Qiang Yu wrote:
> > > The driver currently supports two PERST# and PHY DT configuration. In one
> > > case, PHY, PERST#, are described in the RC node. In the other case, they
> > > are described in the RP node.
> > > 
> > > A mixed setup is not supported. One common example is PHY on the RP node
> > > while PERST# remains on the RC node. In that case the driver goes through
> > > the RP parse path, does not find PERST# on RP, and does not report an
> > > error because PERST# is optional. Probe can then succeed silently while
> > > PERST# is left uncontrolled, and PCIe endpoints fails to work later. This
> > > silent probe success makes debugging difficult.
> > > 
> > > Handle this mixed case in the RP parse path by checking whether PERST# is
> > > present on RC and, if so, using the RC PERST# GPIO for RP ports while
> > > keeping RP parsing for PHY. Emit a warning to indicate mixed DT content so
> > > it can be fixed.
> > > 
> > > This keeps mixed systems functional and makes the configuration issue
> > > visible instead of failing later at endpoint bring-up.
> > > 
> > > Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > > Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> > > ---
> > >   drivers/pci/controller/dwc/pcie-qcom.c | 15 +++++++++++++++
> > >   1 file changed, 15 insertions(+)
> > > 
> > > @@ -1778,6 +1785,14 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
> > >   	struct device *dev = pcie->pci->dev;
> > >   	int ret = -ENODEV;
> > > +	if (of_find_property(dev->of_node, "perst-gpios", NULL)) {
> > > +		pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
> > > +		if (IS_ERR(pcie->reset))
> > > +			return PTR_ERR(pcie->reset);
> > > +
> > > +		dev_warn(dev, "Reusing PERST# from Root Complex node. DT needs to be fixed!\n");
> > This patch breaks legacy DTs (I stumbled on it on SM8350 HDK), because
> > now devm_gpiod_get_optional() in qcom_pcie_parse_legacy_binding()
> > returns -EBUSY (as the GPIO is already requested here). I'll send a
> > patch fixing the issue, but it's sad to see existing platforms being
> > broken for the sake of undeclared "mixed" case.
> 
> Dmitry,
> 
> I have sent[1] fix for this issue and it is squashed with the offending
> commit itself[2] and part of controller/dwc-qcom in the PCI tree.

Ack, thanks

> 
> [1] https://lore.kernel.org/linux-arm-msm/20260526-fix_perst_gpio_handling-v1-1-9170507bb4e9@oss.qualcomm.com/T/#u
> 
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=controller/dwc-qcom&id=1a23bcb452d95f099e530414504c0d99ee076b3f
> 

-- 
With best wishes
Dmitry