Re: [PATCH v2 2/4] PCI: qcom: Keep PERST# GPIO state as-is during probe - sashiko-bot

From: sashiko-bot@kernel.org
To: "Krishna Chaitanya Chundru" <krishna.chundru@oss.qualcomm.com>
Cc: linux-phy@lists.infradead.org, vkoul@kernel.org,
	neil.armstrong@linaro.org, linux-pci@vger.kernel.org,
	olteanv@gmail.com
Subject: Re: [PATCH v2 2/4] PCI: qcom: Keep PERST# GPIO state as-is during probe
Date: Thu, 21 May 2026 13:43:15 +0000	[thread overview]
Message-ID: <20260521134316.8B9821F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260521-link_retain-v2-2-08ed448b081c@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Silent hardware control failure due to incorrect GPIO direction when using `GPIOD_ASIS`.
--

commit b4b7937282f9b2dc2dff52265ec3936b1024af9d
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

PCI: qcom: Keep PERST# GPIO state as-is during probe

This commit changes the PERST# GPIO request to use GPIOD_ASIS instead of
GPIOD_OUT_HIGH. This preserves the existing GPIO state configured by the
bootloader or firmware to avoid unintentionally asserting reset during
probe.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index af6bf5cce65be..bfe873cbf44f6 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1707,7 +1707,7 @@ static int qcom_pcie_parse_perst(struct qcom_pcie *pcie,
>  		goto parse_child_node;
>  
>  	reset = devm_fwnode_gpiod_get(dev, of_fwnode_handle(np), "reset",
> -				      GPIOD_OUT_HIGH, "PERST#");
> +				      GPIOD_ASIS, "PERST#");

[Severity: High]
If the bootloader leaves the PERST# pin floating or as an input, will
this cause gpiod_set_value_cansleep() to silently fail later?

When using GPIOD_ASIS, if the pin was not previously configured as an
output, GPIOD_FLAG_IS_OUT will not be set in the GPIO descriptor.

Later in qcom_pcie_host_init(), the driver attempts to toggle the reset
pin by calling gpiod_set_value_cansleep() via __qcom_pcie_perst_assert().

Because gpiod_set_value_cansleep() enforces directionality, it will
return -EPERM if the pin is not configured as an output. Since
__qcom_pcie_perst_assert() ignores the return value of
gpiod_set_value_cansleep(), the reset line might never actually be driven.

Could this break PCIe link initialization on platforms where the
bootloader hasn't set the GPIO direction to output?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-link_retain-v2-0-08ed448b081c@oss.qualcomm.com?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy