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

[Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>]

On 7/23/2026 4:24 PM, Praveen Talari wrote:
> Hi mukesh
> 
> On 23-07-2026 15:47, Mukesh Savaliya wrote:
>> 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.
> 
> We should not get it and not go further since you are already enabled in 
> system_resume_no_irq().
> 
> I just need to understand why we need to proceed when we get -EACCESS as 
> runtime PM .
> 

ret = 0 was added because during early system resume, -EACCES from 
pm_runtime_get_sync() indicates runtime PM is disabled, not that the 
controller is unusable; treating it as an error would incorrectly reject 
valid I2C transfers occurring during resume.

Other than early PM time, we are not going to get -EACESS. So i don't 
see any case where this will be a problem.
>>
>> 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.
>>
>>>> +    }
>> [...]