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

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

[Abel Vesa]

The RESX GPIO may be provided by a sleeping GPIO controller, such as an
I2C or SPI I/O expander. Calling gpiod_set_value() for such a GPIO
triggers the following warning:

  WARNING: ... drivers/gpio/gpiolib.c:4030 at gpiod_set_value()

The TC9563 power-on and power-off callbacks already sleep while enabling
or disabling regulators and while waiting for oscillator stabilization.
Use gpiod_set_value_cansleep() for RESX so it can be driven without
triggering the warning.

Fixes: 4c9c7be47310 ("PCI: pwrctrl: Add power control driver for TC9563")
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
 drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
index fd53a7db9875..1b7718d07d15 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);
 
 	fsleep(TC9563_OSC_STAB_DELAY_US);
 

---
base-commit: 4b18edbd8e70f7e6860d56370f13244896d0f95c
change-id: 20260825-pci-pwrctl-tc9563-use-cansleep-for-resx-f6e1f65547fc

Best regards,
--  
Abel Vesa <abel.vesa@oss.qualcomm.com>

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

[sashiko-bot]

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

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

[Abel Vesa]

On 26-08-25 11:52:46, sashiko-bot@kernel.org wrote:
> 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().

Duly noted.

Needs to be addressed separately.

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

[Bartosz Golaszewski]

On Tue, 25 Aug 2026 13:41:31 +0200, Abel Vesa <abel.vesa@oss.qualcomm.com> said:
> The RESX GPIO may be provided by a sleeping GPIO controller, such as an
> I2C or SPI I/O expander. Calling gpiod_set_value() for such a GPIO
> triggers the following warning:
>
>   WARNING: ... drivers/gpio/gpiolib.c:4030 at gpiod_set_value()
>
> The TC9563 power-on and power-off callbacks already sleep while enabling
> or disabling regulators and while waiting for oscillator stabilization.
> Use gpiod_set_value_cansleep() for RESX so it can be driven without
> triggering the warning.
>
> Fixes: 4c9c7be47310 ("PCI: pwrctrl: Add power control driver for TC9563")
> Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> ---

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>