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

[Praveen Talari <praveen.talari@oss.qualcomm.com>]

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 .

>
> 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.
>
>>> +    }
> [...]