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

[Carlos Song (OSS)]

> -----Original Message-----
> From: Mukesh Savaliya <mukesh.savaliya at oss.qualcomm.com>
> Sent: Thursday, May 21, 2026 8:40 PM
> To: Carlos Song (OSS) <carlos.song at oss.nxp.com>; Mukesh Savaliya
> <mukesh.savaliya at oss.qualcomm.com>; o.rempel at pengutronix.de;
> kernel at pengutronix.de; andi.shyti at kernel.org; Frank Li <frank.li at nxp.com>;
> s.hauer at pengutronix.de; festevam at gmail.com; Carlos Song
> <carlos.song at nxp.com>; Bough Chen <haibo.chen at nxp.com>
> Cc: linux-i2c at vger.kernel.org; imx at lists.linux.dev;
> linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> stable at 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 at oss.qualcomm.com>
> >> Sent: Thursday, May 21, 2026 7:14 PM
> >> To: Carlos Song (OSS) <carlos.song at oss.nxp.com>; Mukesh Savaliya
> >> <mukesh.savaliya at oss.qualcomm.com>; o.rempel at pengutronix.de;
> >> kernel at pengutronix.de; andi.shyti at kernel.org; Frank Li
> >> <frank.li at nxp.com>; s.hauer at pengutronix.de; festevam at gmail.com;
> >> Carlos Song <carlos.song at nxp.com>; Bough Chen <haibo.chen at nxp.com>
> >> Cc: linux-i2c at vger.kernel.org; imx at lists.linux.dev;
> >> linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> >> stable at 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 at oss.qualcomm.com>
> >>>>>> Sent: Thursday, May 21, 2026 3:40 PM
> >>>>>> To: Carlos Song (OSS) <carlos.song at oss.nxp.com>;
> >>>>>> o.rempel at pengutronix.de; kernel at pengutronix.de;
> >>>>>> andi.shyti at kernel.org; Frank Li <frank.li at nxp.com>;
> >>>>>> s.hauer at pengutronix.de; festevam at gmail.com; Carlos Song
> >>>>>> <carlos.song at nxp.com>; Bough Chen <haibo.chen at nxp.com>
> >>>>>> Cc: linux-i2c at vger.kernel.org; imx at lists.linux.dev;
> >>>>>> linux-arm-kernel at lists.infradead.org;
> >>>>>> linux-kernel at vger.kernel.org; stable at 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 at 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?

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