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

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

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.

> 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.

>>> Carlos Song
>>
>