* [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered down
@ 2026-05-20 10:15 Carlos Song (OSS)
2026-05-21 7:40 ` Mukesh Savaliya
0 siblings, 1 reply; 10+ messages in thread
From: Carlos Song (OSS) @ 2026-05-20 10:15 UTC (permalink / raw)
To: o.rempel, kernel, andi.shyti, Frank.Li, s.hauer, festevam,
carlos.song, haibo.chen
Cc: linux-i2c, imx, linux-arm-kernel, linux-kernel, stable
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.
Fixes: 358025ac091e ("i2c: imx: make controller available until system suspend_noirq() and from resume_noirq()")
Cc: stable@vger.kernel.org
Signed-off-by: Carlos Song <carlos.song@nxp.com>
---
Change for v3:
- Add hrtimer_cancel in i2c_imx_suspend_noirq to cancel slave_timer for
safe suspend in i2c slave mode.
Change for v2:
- Call i2c_mark_adapter_suspended() before pm_runtime_force_suspend()
to prevent potential deadlock if a transfer is active during suspend.
- Roll back with i2c_mark_adapter_resumed() if pm_runtime_force_suspend()
fails.
---
drivers/i2c/busses/i2c-imx.c | 41 ++++++++++++++++++++++++++++++++++--
1 file changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index a208fefd3c3b..d651ade86267 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1913,6 +1913,43 @@ static int i2c_imx_runtime_resume(struct device *dev)
return ret;
}
+static int __maybe_unused i2c_imx_suspend_noirq(struct device *dev)
+{
+ struct imx_i2c_struct *i2c_imx = dev_get_drvdata(dev);
+ int ret;
+
+ i2c_mark_adapter_suspended(&i2c_imx->adapter);
+
+ /*
+ * Cancel the slave timer before powering down to prevent
+ * i2c_imx_slave_timeout() from accessing hardware registers
+ * while the clock is disabled.
+ */
+ hrtimer_cancel(&i2c_imx->slave_timer);
+
+ ret = pm_runtime_force_suspend(dev);
+ if (ret) {
+ i2c_mark_adapter_resumed(&i2c_imx->adapter);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int __maybe_unused i2c_imx_resume_noirq(struct device *dev)
+{
+ struct imx_i2c_struct *i2c_imx = dev_get_drvdata(dev);
+ int ret;
+
+ ret = pm_runtime_force_resume(dev);
+ if (ret)
+ return ret;
+
+ i2c_mark_adapter_resumed(&i2c_imx->adapter);
+
+ return 0;
+}
+
static int i2c_imx_suspend(struct device *dev)
{
/*
@@ -1946,8 +1983,8 @@ static int i2c_imx_resume(struct device *dev)
}
static const struct dev_pm_ops i2c_imx_pm_ops = {
- NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
- pm_runtime_force_resume)
+ NOIRQ_SYSTEM_SLEEP_PM_OPS(i2c_imx_suspend_noirq,
+ i2c_imx_resume_noirq)
SYSTEM_SLEEP_PM_OPS(i2c_imx_suspend, i2c_imx_resume)
RUNTIME_PM_OPS(i2c_imx_runtime_suspend, i2c_imx_runtime_resume, NULL)
};
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered down 2026-05-20 10:15 [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered down Carlos Song (OSS) @ 2026-05-21 7:40 ` Mukesh Savaliya 2026-05-21 8:27 ` Carlos Song (OSS) 0 siblings, 1 reply; 10+ messages in thread From: Mukesh Savaliya @ 2026-05-21 7:40 UTC (permalink / raw) To: Carlos Song (OSS), o.rempel, kernel, andi.shyti, Frank.Li, s.hauer, festevam, carlos.song, haibo.chen Cc: linux-i2c, imx, linux-arm-kernel, linux-kernel, stable 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. > 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" ? > Fixes: 358025ac091e ("i2c: imx: make controller available until system suspend_noirq() and from resume_noirq()") > Cc: stable@vger.kernel.org > Signed-off-by: Carlos Song <carlos.song@nxp.com> > --- > Change for v3: > - Add hrtimer_cancel in i2c_imx_suspend_noirq to cancel slave_timer for > safe suspend in i2c slave mode. > Change for v2: > - Call i2c_mark_adapter_suspended() before pm_runtime_force_suspend() > to prevent potential deadlock if a transfer is active during suspend. > - Roll back with i2c_mark_adapter_resumed() if pm_runtime_force_suspend() > fails. > --- [...] ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered down 2026-05-21 7:40 ` Mukesh Savaliya @ 2026-05-21 8:27 ` Carlos Song (OSS) 2026-05-21 10:16 ` Mukesh Savaliya 0 siblings, 1 reply; 10+ messages in thread From: Carlos Song (OSS) @ 2026-05-21 8:27 UTC (permalink / raw) To: Mukesh Savaliya, Carlos Song (OSS), o.rempel@pengutronix.de, kernel@pengutronix.de, andi.shyti@kernel.org, Frank Li, s.hauer@pengutronix.de, festevam@gmail.com, Carlos Song, Bough Chen 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 > -----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 > > [You don't often get email from mukesh.savaliya@oss.qualcomm.com. Learn > why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > 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. > > > 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. 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? Carlos Song > > Fixes: 358025ac091e ("i2c: imx: make controller available until system > > suspend_noirq() and from resume_noirq()") > > Cc: stable@vger.kernel.org > > Signed-off-by: Carlos Song <carlos.song@nxp.com> > > --- > > Change for v3: > > - Add hrtimer_cancel in i2c_imx_suspend_noirq to cancel slave_timer for > > safe suspend in i2c slave mode. > > Change for v2: > > - Call i2c_mark_adapter_suspended() before > pm_runtime_force_suspend() > > to prevent potential deadlock if a transfer is active during suspend. > > - Roll back with i2c_mark_adapter_resumed() if > pm_runtime_force_suspend() > > fails. > > --- > > [...] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered down 2026-05-21 8:27 ` Carlos Song (OSS) @ 2026-05-21 10:16 ` Mukesh Savaliya 2026-05-21 10:51 ` Carlos Song (OSS) 0 siblings, 1 reply; 10+ messages in thread From: Mukesh Savaliya @ 2026-05-21 10:16 UTC (permalink / raw) To: Carlos Song (OSS), Mukesh Savaliya, o.rempel@pengutronix.de, kernel@pengutronix.de, andi.shyti@kernel.org, Frank Li, s.hauer@pengutronix.de, festevam@gmail.com, Carlos Song, Bough Chen 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 Thanks Carlos ! On 5/21/2026 1:57 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 >> >> [You don't often get email from mukesh.savaliya@oss.qualcomm.com. Learn >> why this is important at https://aka.ms/LearnAboutSenderIdentification ] >> >> 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. > > Carlos Song ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered down 2026-05-21 10:16 ` Mukesh Savaliya @ 2026-05-21 10:51 ` Carlos Song (OSS) 2026-05-21 11:14 ` Mukesh Savaliya 0 siblings, 1 reply; 10+ messages in thread From: Carlos Song (OSS) @ 2026-05-21 10:51 UTC (permalink / raw) To: Mukesh Savaliya, Carlos Song (OSS), o.rempel@pengutronix.de, kernel@pengutronix.de, andi.shyti@kernel.org, Frank Li, s.hauer@pengutronix.de, festevam@gmail.com, Carlos Song, Bough Chen 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 > -----Original Message----- > From: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com> > Sent: Thursday, May 21, 2026 6:17 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 > > [You don't often get email from mukesh.savaliya@oss.qualcomm.com. Learn > why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > Thanks Carlos ! > > On 5/21/2026 1:57 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 > >> > >> [You don't often get email from mukesh.savaliya@oss.qualcomm.com. > >> Learn why this is important at > >> https://aka.ms/LearnAboutSenderIdentification ] > >> > >> 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. 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 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? > > > > Carlos Song > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered down 2026-05-21 10:51 ` Carlos Song (OSS) @ 2026-05-21 11:14 ` Mukesh Savaliya 2026-05-21 12:02 ` Carlos Song (OSS) 0 siblings, 1 reply; 10+ messages in thread From: Mukesh Savaliya @ 2026-05-21 11:14 UTC (permalink / raw) To: Carlos Song (OSS), Mukesh Savaliya, o.rempel@pengutronix.de, kernel@pengutronix.de, andi.shyti@kernel.org, Frank Li, s.hauer@pengutronix.de, festevam@gmail.com, Carlos Song, Bough Chen 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 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 >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered down 2026-05-21 11:14 ` Mukesh Savaliya @ 2026-05-21 12:02 ` Carlos Song (OSS) 2026-05-21 12:39 ` Mukesh Savaliya 0 siblings, 1 reply; 10+ messages in thread From: Carlos Song (OSS) @ 2026-05-21 12:02 UTC (permalink / raw) To: Mukesh Savaliya, Carlos Song (OSS), o.rempel@pengutronix.de, kernel@pengutronix.de, andi.shyti@kernel.org, Frank Li, s.hauer@pengutronix.de, festevam@gmail.com, Carlos Song, Bough Chen 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 > -----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. > > 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 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? > >> > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered down 2026-05-21 12:02 ` Carlos Song (OSS) @ 2026-05-21 12:39 ` Mukesh Savaliya 2026-05-21 14:49 ` Carlos Song (OSS) 0 siblings, 1 reply; 10+ messages in thread From: Mukesh Savaliya @ 2026-05-21 12:39 UTC (permalink / raw) To: Carlos Song (OSS), Mukesh Savaliya, o.rempel@pengutronix.de, kernel@pengutronix.de, andi.shyti@kernel.org, Frank Li, s.hauer@pengutronix.de, festevam@gmail.com, Carlos Song, Bough Chen 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 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 ?] > > 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 ! >>>> >>> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered down 2026-05-21 12:39 ` Mukesh Savaliya @ 2026-05-21 14:49 ` Carlos Song (OSS) 2026-05-22 7:20 ` Mukesh Savaliya 0 siblings, 1 reply; 10+ messages in thread From: Carlos Song (OSS) @ 2026-05-21 14:49 UTC (permalink / raw) To: Mukesh Savaliya, Carlos Song (OSS), o.rempel@pengutronix.de, kernel@pengutronix.de, andi.shyti@kernel.org, Frank Li, s.hauer@pengutronix.de, festevam@gmail.com, Carlos Song, Bough Chen 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 > -----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? > > > > 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 ! > > >>>> > >>> > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered down 2026-05-21 14:49 ` Carlos Song (OSS) @ 2026-05-22 7:20 ` Mukesh Savaliya 0 siblings, 0 replies; 10+ messages in thread From: Mukesh Savaliya @ 2026-05-22 7:20 UTC (permalink / raw) To: Carlos Song (OSS), o.rempel@pengutronix.de, kernel@pengutronix.de, andi.shyti@kernel.org, Frank Li, s.hauer@pengutronix.de, festevam@gmail.com, Carlos Song, Bough Chen 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 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 ! >> >>>>>> >>>>> >>> > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-05-22 7:20 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-20 10:15 [PATCH v3] i2c: imx: mark I2C adapter when hardware is powered down Carlos Song (OSS) 2026-05-21 7:40 ` Mukesh Savaliya 2026-05-21 8:27 ` Carlos Song (OSS) 2026-05-21 10:16 ` Mukesh Savaliya 2026-05-21 10:51 ` Carlos Song (OSS) 2026-05-21 11:14 ` Mukesh Savaliya 2026-05-21 12:02 ` Carlos Song (OSS) 2026-05-21 12:39 ` Mukesh Savaliya 2026-05-21 14:49 ` Carlos Song (OSS) 2026-05-22 7:20 ` Mukesh Savaliya
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox