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

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

[Carlos Song (OSS)]

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: 1ee867e465c1 ("i2c: imx-lpi2c: add target mode support")
Cc: stable@vger.kernel.org
Signed-off-by: Carlos Song <carlos.song@nxp.com>
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index a01c23696481..bf8c1ce1c7ff 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -1635,7 +1635,16 @@ static int __maybe_unused lpi2c_runtime_resume(struct device *dev)
 
 static int __maybe_unused lpi2c_suspend_noirq(struct device *dev)
 {
-	return pm_runtime_force_suspend(dev);
+	struct lpi2c_imx_struct *lpi2c_imx = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_force_suspend(dev);
+	if (ret)
+		return ret;
+
+	i2c_mark_adapter_suspended(&lpi2c_imx->adapter);
+
+	return 0;
 }
 
 static int __maybe_unused lpi2c_resume_noirq(struct device *dev)
@@ -1655,6 +1664,8 @@ static int __maybe_unused lpi2c_resume_noirq(struct device *dev)
 	if (lpi2c_imx->target)
 		lpi2c_imx_target_init(lpi2c_imx);
 
+	i2c_mark_adapter_resumed(&lpi2c_imx->adapter);
+
 	return 0;
 }
 
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect ordering of adapter suspension and hardware power-down creates a race condition leading to deadlocks and kernel panics.
--

commit 4f51dfc32f43d5578829b4cc2fac1f8c956c3ca8
Author: Carlos Song <carlos.song@nxp.com>
Subject: i2c: imx-lpi2c: mark I2C adapter when hardware is powered down

This commit updates the suspend and resume routines to mark the I2C
adapter as suspended or resumed. This is intended to block transfers
while the hardware is powered down and prevent hangs.

> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index a01c236964819..bf8c1ce1c7ff0 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -1635,7 +1635,16 @@ static int __maybe_unused lpi2c_runtime_resume(struct device *dev)
>  
>  static int __maybe_unused lpi2c_suspend_noirq(struct device *dev)
>  {
> -	return pm_runtime_force_suspend(dev);
> +	struct lpi2c_imx_struct *lpi2c_imx = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = pm_runtime_force_suspend(dev);
> +	if (ret)
> +		return ret;
> +
> +	i2c_mark_adapter_suspended(&lpi2c_imx->adapter);

Could this ordering cause a deadlock and external abort if an I2C transfer
is currently active during suspend?

When pm_runtime_force_suspend is called, it disables the controller clocks.
If a transfer is ongoing, the hardware will stop responding and the
transfer will hang waiting for a completion timeout.

Then, i2c_mark_adapter_suspended attempts to acquire the root adapter lock,
which is still held by the hanging transfer, causing the suspend path to
block.

When the hung transfer eventually times out, its error handling path will
try to reset or abort the transfer by writing to the hardware registers.
Since the clocks were already disabled by pm_runtime_force_suspend,
wouldn't accessing these registers trigger a synchronous external abort
and panic the kernel?

Should i2c_mark_adapter_suspended be called before pm_runtime_force_suspend
so it can safely acquire the bus lock, wait for active transfers to finish,
and reject new transfers before the hardware is powered down?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519104106.2794103-1-carlos.song@oss.nxp.com?part=1