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

Hi Praveen,

On 7/23/2026 3:32 PM, Praveen Talari wrote:
> Hi Mukesh
> 
[...]

>> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
>> index 96dbf04138be..4bc00922cd97 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;
> 
> Here it is clear that pm_runtime_get_sync() is returning -EACCES from 
> the runtime PM framework,
> 
> which indicates that the request was rejected before the runtime resume 
> path could enable any resources.
> 
>  From the rpm_resume() implementation:
> 
> static int rpm_resume(struct device *dev, int rpmflags) __releases(&dev- 
>  >power.lock) __acquires(&dev->power.lock) { [...] repeat: if (dev- 
>  >power.runtime_error) { retval = -EINVAL; } else if (dev- 
>  >power.disable_depth > 0) { [...] else retval = -EACCES; } if (retval) 
> goto out; [...] out: if (parent && !dev->power.irq_safe) 
> { spin_unlock_irq(&dev->power.lock); pm_runtime_put(parent); 
> spin_lock_irq(&dev->power.lock); } trace_rpm_return_int(dev, _THIS_IP_, 
> retval); return retval; }
> 
> The -EACCES error is returned when runtime PM is disabled (disable_depth 
>  > 0),
> 
> causing the function to exit without invoking the device's runtime 
> resume callback.
> 
> As a result, no resource enablement is performed by the PM framework.
> 
> 
> In your change, the error is effectively converted to ret = 0 and 
> execution continues.
> 
> How can we guarantee that the required resources have been enabled in 
> this scenario?
> 
> More importantly, where are those resources expected to be enabled if 
> the runtime PM resume path was never executed?
> 

This was answered in V6 already on 7/3 exactly at same place.
Pasting below.

==
 > Why we are checking specific error code here? Why can't we use the 
below error check directly?
 > if get sync itself is failed with pm runtime disabled then why we are 
going ahead by making ret = 0 here? How you will make sure resources are 
enabled?

This reason is also mentioned in the commit log. we surely get -EACCESS 
as runtime PM is disabled during no_irq resume phase. We still need to 
serve the transfer, hence we need to override it.

Review geni_i2c_resume_noirq() to know what all we do to enable 
resources. That's guaranteed and tested with system suspend/resume test 
back to back and PCIe could do i2c transfer successfully.
==
> 
> Thanks,
> 
> Praveen Talari
> 
Do not keep this thanks/regards here. only leave the comments.

>> +	}
[...]