Re: [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered down

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

Thanks Carlos !

On 5/21/2026 8:19 PM, Carlos Song (OSS) wrote:
> 
> 
>> -----Original Message-----
>> From: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>
>> Sent: Thursday, May 21, 2026 8:40 PM
>> To: Carlos Song (OSS) <carlos.song@oss.nxp.com>; Mukesh Savaliya
>> <mukesh.savaliya@oss.qualcomm.com>; o.rempel@pengutronix.de;
>> kernel@pengutronix.de; andi.shyti@kernel.org; Frank Li <frank.li@nxp.com>;
>> s.hauer@pengutronix.de; festevam@gmail.com; Carlos Song
>> <carlos.song@nxp.com>; Bough Chen <haibo.chen@nxp.com>
>> Cc: linux-i2c@vger.kernel.org; imx@lists.linux.dev;
>> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
>> stable@vger.kernel.org
>> Subject: Re: [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered
>> down
>>
>>
>>
>> On 5/21/2026 5:32 PM, Carlos Song (OSS) wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>
>>>> Sent: Thursday, May 21, 2026 7:14 PM
>>>> To: Carlos Song (OSS) <carlos.song@oss.nxp.com>; Mukesh Savaliya
>>>> <mukesh.savaliya@oss.qualcomm.com>; o.rempel@pengutronix.de;
>>>> kernel@pengutronix.de; andi.shyti@kernel.org; Frank Li
>>>> <frank.li@nxp.com>; s.hauer@pengutronix.de; festevam@gmail.com;
>>>> Carlos Song <carlos.song@nxp.com>; Bough Chen <haibo.chen@nxp.com>
>>>> Cc: linux-i2c@vger.kernel.org; imx@lists.linux.dev;
>>>> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
>>>> stable@vger.kernel.org
>>>> Subject: Re: [PATCH v3] i2c: imx: mark I2C adapter when hardware is
>>>> powered down
>>>>
>>>>
>>>> On 5/21/2026 4:21 PM, Carlos Song (OSS) wrote:
>>>>
>>>> [...]
>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>
>>>>>>>> Sent: Thursday, May 21, 2026 3:40 PM
>>>>>>>> To: Carlos Song (OSS) <carlos.song@oss.nxp.com>;
>>>>>>>> o.rempel@pengutronix.de; kernel@pengutronix.de;
>>>>>>>> andi.shyti@kernel.org; Frank Li <frank.li@nxp.com>;
>>>>>>>> s.hauer@pengutronix.de; festevam@gmail.com; Carlos Song
>>>>>>>> <carlos.song@nxp.com>; Bough Chen <haibo.chen@nxp.com>
>>>>>>>> Cc: linux-i2c@vger.kernel.org; imx@lists.linux.dev;
>>>>>>>> linux-arm-kernel@lists.infradead.org;
>>>>>>>> linux-kernel@vger.kernel.org; stable@vger.kernel.org
>>>>>>>> Subject: Re: [PATCH v3] i2c: imx: mark I2C adapter when hardware
>>>>>>>> is powered down
>>>>>>>>
>>>>>>>> Hi Carlos,
>>>>>>>>
>>>>>>>> On 5/20/2026 3:45 PM, Carlos Song (OSS) wrote:
>>>>>>>>> From: Carlos Song <carlos.song@nxp.com>
>>>>>>>>>
>>>>>>>>> Mark the I2C adapter as suspended during system suspend to block
>>>>>>>>> further transfers, and resume it on system resume. This prevents
>>>>>>>>> potential hangs when the hardware is powered down but clients
>>>>>>>>> still attempt
>>>>>>>> I2C transfers.
>>>>>>>>>
>>>>>> what was the reason of this hang ? I was thinking you don't have
>>>>>> interrupts working when client requested transfer but adapter was
>>>>>> suspended. Please correct me if wrong.
>>>>>>
>>>>>> And it would be good to mention the actual problem and why/how it
>>>> occurred.
>>>>>>>> Code changes looks fine to me but have comment on commit log.
>>>>>>>>
>>>>>>>> It seems, you are adding support of _noirq() callbacks to allow
>>>>>>>> transfers during suspend/resume noirq phase of PM.
>>>>>>>>
>>>>>>>> Would it make sense if you can write "Replace system PM callbacks
>>>>>>>> with noirq PM callbacks" OR "Allow transfers during _noirq phase
>>>>>>>> of the PM ops" instead of "mark I2C adapter when hardware is
>>>>>>>> powered
>>>>>> down" ?
>>>>>>>>
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Thank you for your comments!
>>>>>>>
>>>>>>> But this patch is added is not for support noirq PM callback or
>>>>>>> transfer in noirq
>>>>>> phase.
>>>>>>>
>>>>>> Okay, may be actual problem description can help me.
>>>>>>> In fact, this fix is to mark the I2C adapter as suspended during
>>>>>>> system noirq suspend to block further transfers, and resume it on
>>>>>>> system noirq resume. This is to prohibit I2C device calling the
>>>>>>> I2C controller after the system noirq suspend and before noirq
>>>>>>> resume, because at
>>>>>> this time the I2C instance is powered off or the clock is disabled
>>>>>> ... So I want to keep current commit. How do you think?
>>>>>> completely Makes sense. Please help add how this problem occurred
>>>>>> and
>>>> why ?
>>>>>> So the change/fix will be good to understand against it.
>>>>>
>>>>> Hi,
>>>>>
>>>>> In some I.MX platform, some I2C devices will keep a work queue all
>>>>> time, the work queue will trigger I2C xfer every once in a while,
>>>>> but the work
>>>> queue shouldn't be free in system suspend.
>>>>>
>>>>
>>>> work queue has transfers queued even if system is suspended ? IMO,
>>>> the client i2c devices should not let system go to suspend.
>>>>
>>>
>>> Hi Mukesh,
>>>
>>> Thank you for the detailed discussion.
>>>
>>> Yes, I totally agree that I2C client drivers should ideally stop
>>> issuing transfers when the system is suspending.
>>>
>>> However, in practice there are many different I2C clients, and not all
>>> of them strictly adhere to this requirement. Some clients may still
>>> trigger transfers through workqueues or deferred contexts during the
>>> suspend/resume window.
>>>
>>> Therefore, adding this protection at the I2C controller side helps to
>>> avoid unexpected accesses when the hardware resources are unavailable,
>>> making the system more robust.
>>>
>>
>> Agreed !
>>
>>>>> Within a very short time window, possibly from noirq_suspend to the
>>>>> system actually being suspended, or possibly from the system
>>>>> starting to resume to before noirq_resume, this work queue will
>>>>> trigger an I2C transfer, and at this time the I2C controller's clk
>>>>> and pinctrl have not yet been restored, reading and
>>>>
>>>> Right, this kind of explains the problem to me. I think you are
>>>> trying to serve i2c transfers when your resources(clk, pinctrl) are
>>>> not turned ON and also interrupt remains disabled. And that's why you
>>>> need to add
>>>> _noir() PM callbacks supports along with IRQF_NO_SUSPEND |
>>>> IRQF_EARLY_RESUME flags.
>>>>
>>>>> writing I2C registers causes the system to hang. This patch make all
>>>>> I2C operations are performed in a safe hardware state.
>>>>>
>>>>> Is it better if I add these comment to patch commit log?
>>>>>>>
>>>> if my latest comments makes sense against the issue, you may write
>>>> accordingly. if i am wrong, then your explanation makes sense. Cause
>>>> of the hang needs to be clearly mention int the commit log in your next
>> patch.
>>>>
>>>
>>> Based on our discussion, I have updated the commit log as below:
>>>
>>> On some i.MX platforms, certain I2C client drivers keep a periodic
>>> workqueue which continues to trigger I2C transfers.
>>>
>>> During system suspend/resume, there exists a time window between:
>>>     - noirq_suspend and full suspend
>>>     - resume start and noirq_resume
>>
>> - noirq_resume and resume start [Just opposite ?]
>>
> 
> Sorry, the expression is ambiguous.
> 
> I will update the commit log to:
> 
> During system suspend/resume, there exists a time window between:
>    - suspend_noirq and the system entering suspend
>    - the system starting to resume and resume_noirq
> 
> Does this look good to you?
Yes, looks good.
> 
>>>
>>> In this window, the I2C controller resources such as clock and pinctrl
>>> may already be disabled or not yet restored.
>>>
>>> If a workqueue triggers an I2C transfer in this period, the driver
>>> attempts to access I2C registers while the hardware resources are
>>> unavailable, which may lead to system hang.
>>>
>>> Mark the I2C adapter as suspended during noirq suspend and block new
>>> transfers until resume, ensuring that I2C transfers are only issued
>>> when hardware resources are available.
>>>
>>> Does this look good to you?
>>>
>> Looks good, Thanks !
>>
>>>>>>
>>>>>
>>>
>