Re: [PATCH] PCI/pwrctrl: tc9563: Use gpiod_set_value_cansleep() for RESX

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] I2C/SPI transfers during the noirq phase of system suspend/resume will timeout and fail if the host controller relies on interrupts.
--

--- Patch [1]: [PATCH] PCI/pwrctrl: tc9563: Use gpiod_set_value_cansleep() for RESX ---
commit cc5d1431bebcee3e75ac6e1b5150b5971fe40f41
Author: Abel Vesa <abel.vesa@oss.qualcomm.com>

PCI/pwrctrl: tc9563: Use gpiod_set_value_cansleep() for RESX
    
This patch changes gpiod_set_value() to gpiod_set_value_cansleep() for the
RESX GPIO in the TC9563 power-on and power-off callbacks. This prevents
warnings when the GPIO is provided by a sleeping controller, such as an I2C
or SPI I/O expander.

> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> index fd53a7db9875d..1b7718d07d15d 100644
> --- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> @@ -481,7 +481,7 @@ static int tc9563_pwrctrl_power_off(struct pci_pwrctrl *pwrctrl)
>  	struct tc9563_pwrctrl *tc9563 = container_of(pwrctrl,
>  					    struct tc9563_pwrctrl, pwrctrl);
>  
> -	gpiod_set_value(tc9563->reset_gpio, 1);
> +	gpiod_set_value_cansleep(tc9563->reset_gpio, 1);
>  
>  	regulator_bulk_disable(ARRAY_SIZE(tc9563->supplies), tc9563->supplies);
>  
> @@ -501,7 +501,7 @@ static int tc9563_pwrctrl_power_on(struct pci_pwrctrl *pwrctrl)
>  	if (ret < 0)
>  		return dev_err_probe(dev, ret, "cannot enable regulators\n");
>  
> -	gpiod_set_value(tc9563->reset_gpio, 0);
> +	gpiod_set_value_cansleep(tc9563->reset_gpio, 0);

[Severity: High]
This isn't a bug introduced by this patch, but it makes a pre-existing issue
more apparent by acknowledging the GPIO's I2C/SPI dependency.

Will I2C/SPI transfers during the noirq phase of system suspend/resume timeout
and fail if the host controller relies on interrupts?

During system suspend/resume, the PCIe host controller executes
qcom_pcie_suspend_noirq() or qcom_pcie_resume_noirq(), which eventually call
pci_pwrctrl_power_off_devices() or pci_pwrctrl_power_on_devices(). These
invoke tc9563_pwrctrl_power_off() and tc9563_pwrctrl_power_on().

During the noirq phase, device interrupts are disabled system-wide via
suspend_device_irqs(). Because local CPU interrupts remain enabled,
i2c_in_atomic_xfer_mode() evaluates to false, causing the I2C core to use the
standard interrupt-driven master_xfer() rather than master_xfer_atomic().

Since the I2C controller's device interrupt is disabled, won't the transfer
time out and fail, potentially causing a system freeze or suspend/resume
failure?

This would affect both the sleeping GPIO expander and the I2C bulk writes
performed later in tc9563_pwrctrl_power_on() when calling
tc9563_pwrctrl_disable_port().

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825-pci-pwrctl-tc9563-use-cansleep-for-resx-v1-1-0dbe7cfd8200@oss.qualcomm.com?part=1