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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Requesting the PERST# GPIO with GPIOD_ASIS leaves it as an input if not configured by the bootloader, causing subsequent reset toggles to silently fail.
--

--- Patch [2]: [PATCH v3 2/4] PCI: qcom: Keep PERST# GPIO state as-is during probe ---
commit 9e74f48a37d93ad22a1ec71470af58b05c07c912
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 from GPIOD_OUT_HIGH to GPIOD_ASIS
in order to preserve the existing GPIO state configured by the bootloader or
firmware during probe.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1841,7 +1841,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 does not configure the PERST# GPIO as an output, does
requesting it with GPIOD_ASIS leave the pin configured as an input?

If so, later when __qcom_pcie_perst_assert() attempts to toggle the reset:

	gpiod_set_value_cansleep(perst->desc, val);

Will it silently fail to drive the pin, preventing the PCIe link from being
established?

>  	if (IS_ERR(reset)) {
>  		/*
>  		 * FIXME: GPIOLIB currently supports exclusive GPIO access only.

[ ... ]

> @@ -1918,7 +1918,7 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
>  
>  	if (of_find_property(dev->of_node, "perst-gpios", NULL)) {
>  		pcie->reset = devm_gpiod_get_optional(dev, "perst",
> -						      GPIOD_OUT_HIGH);
> +						      GPIOD_ASIS);

[Severity: High]
This appears to have the same issue. If the GPIO defaults to an input and
is not configured by the bootloader, will gpiod_set_value_cansleep()
silently fail to drive the line later?

>  		if (IS_ERR(pcie->reset))
>  			return PTR_ERR(pcie->reset);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-link_retain-v3-0-81a9d187bb61@oss.qualcomm.com?part=2