Re: [PATCH v7 1/2] i2c: qcom-geni: Handle runtime PM disabled state during early resume

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Calling pm_runtime_enable() in the resume_noirq callback causes an unbalanced disable_depth, triggering a kernel warning during early resume.
- [High] Treating -EACCES from pm_runtime_get_sync() as success allows unclocked register accesses on powered-off hardware, risking fatal bus errors.
--

--- Patch [1]: [PATCH v7 1/2] i2c: qcom-geni: Handle runtime PM disabled state during early resume ---
commit 5b7e2f3ccf132073e5ef0e9e9ebe44fb7923a932
Author: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>

i2c: qcom-geni: Handle runtime PM disabled state during early resume

During the noirq resume phase, the GENI I2C controller may receive transfer
requests before runtime PM has been fully restored. This patch treats the
resulting -EACCES error from pm_runtime_get_sync() as a transient condition
and allows the transfer path to proceed. It also manually enables runtime
PM in the resume_noirq callback.

> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 96dbf04138bee..4bc00922cd97b 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -917,6 +917,10 @@ static int geni_i2c_xfer(struct i2c_adapter *adap,
>  	gi2c->err = 0;
>  	reinit_completion(&gi2c->done);
>  	ret = pm_runtime_get_sync(gi2c->se.dev);
> +	if (ret == -EACCES) {
> +		dev_warn(gi2c->se.dev, "Runtime PM is disabled:%d\n", ret);
> +		ret = 0;
> +	}

[Severity: High]
Does treating -EACCES as success risk causing fatal bus errors?

If an I2C client device issues a transfer during the system suspend late
or noirq phases before the controller's noirq resume callback runs,
pm_runtime_get_sync() returns -EACCES because the hardware is in
RPM_SUSPENDED (power domain and clocks are off).

By overriding the error and forcing ret = 0, the synchronization barrier
is bypassed. The driver will then proceed to write to unclocked hardware
registers, which can trigger a synchronous external abort.

[ ... ]

> @@ -1223,7 +1228,12 @@ static int __maybe_unused geni_i2c_resume_noirq(struct device *dev)
>  	if (ret)
>  		return ret;
>  
> +	/* Enforced disable_depth = 0 to actually enable runtime PM during noirq phase */
> +	if (!pm_runtime_enabled(dev))
> +		pm_runtime_enable(dev);
> +
>  	i2c_mark_adapter_resumed(&gi2c->adap);

[Severity: High]
Will this unbalance the disable_depth counter and trigger a kernel warning?

The PM core's device_suspend_late() disables runtime PM, incrementing
disable_depth. Calling pm_runtime_enable() here prematurely decrements
the counter to 0.

When the PM core subsequently calls pm_runtime_enable() in
device_resume_early(), it will detect an underflow and unconditionally
print the "Unbalanced pm_runtime_enable!" warning on every system
resume cycle.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709134623.1724212-1-mukesh.savaliya@oss.qualcomm.com?part=1