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

On 7/24/2026 10:12 AM, Praveen Talari wrote:
> Hi
> 
> On 23-07-2026 17:14, Mukesh Savaliya wrote:
>>
>>
>> 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.
> 
> 
> What is the purpose of enabling runtime PM during system resume? It is 
> not clear how this helps, given that -EACCES can still be returned.
> 

I think you didn't get noirq_resume() change. if you review code, we do 
force_resume(), but that doesn't invoke transfer time runtime_resume() 
via pm_*_get_sync() call because device has runtime PM disabled.

Hence we need to call pm_runtime_enable(). In my testing, have seen that 
driver returning with -EACCESS if not overridden with ret = 0. That's 
the whole point. We need force resume and transfer to serve early resume 
transfer. Usecase demands this transfer without failure.

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