From: Carlos Song <carlos.song@nxp.com>

In i2c_imx_runtime_suspend(), the clock is disabled before switching
the pinctrl state to sleep. If pinctrl_pm_select_sleep_state() fails,
the runtime suspend is aborted but the clock remains disabled, causing
a system crash when the hardware is subsequently accessed.

Fix this by switching the pinctrl state before disabling the clock so
that a pinctrl failure leaves the clock enabled and the hardware
accessible.

In i2c_imx_runtime_resume(), restore the pinctrl state back to sleep
if clk_enable() fails to keep the two consistent.

Fixes: 576eba03c994 ("i2c: imx: switch different pinctrl state in different system power status")
Cc: stable@vger.kernel.org
Signed-off-by: Carlos Song <carlos.song@nxp.com>
---
 drivers/i2c/busses/i2c-imx.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index d651ade86267..54fd5d0e4056 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1892,9 +1892,15 @@ static void i2c_imx_remove(struct platform_device *pdev)
 static int i2c_imx_runtime_suspend(struct device *dev)
 {
 	struct imx_i2c_struct *i2c_imx = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pinctrl_pm_select_sleep_state(dev);
+	if (ret)
+		return ret;
 
 	clk_disable(i2c_imx->clk);
-	return pinctrl_pm_select_sleep_state(dev);
+
+	return 0;
 }
 
 static int i2c_imx_runtime_resume(struct device *dev)
@@ -1907,10 +1913,13 @@ static int i2c_imx_runtime_resume(struct device *dev)
 		return ret;
 
 	ret = clk_enable(i2c_imx->clk);
-	if (ret)
+	if (ret) {
 		dev_err(dev, "can't enable I2C clock, ret=%d\n", ret);
+		pinctrl_pm_select_sleep_state(dev);
+		return ret;
+	}
 
-	return ret;
+	return 0;
 }
 
 static int __maybe_unused i2c_imx_suspend_noirq(struct device *dev)
-- 
2.43.0

On Wed, May 20, 2026 at 06:49:39PM +0800, Carlos Song (OSS) wrote:
> From: Carlos Song <carlos.song@nxp.com>
>
> In i2c_imx_runtime_suspend(), the clock is disabled before switching
> the pinctrl state to sleep. If pinctrl_pm_select_sleep_state() fails,
> the runtime suspend is aborted but the clock remains disabled, causing
> a system crash when the hardware is subsequently accessed.
>
> Fix this by switching the pinctrl state before disabling the clock so
> that a pinctrl failure leaves the clock enabled and the hardware
> accessible.
>
> In i2c_imx_runtime_resume(), restore the pinctrl state back to sleep
> if clk_enable() fails to keep the two consistent.

nit: remove "two", just keep the consistent.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> Fixes: 576eba03c994 ("i2c: imx: switch different pinctrl state in different system power status")
> Cc: stable@vger.kernel.org
> Signed-off-by: Carlos Song <carlos.song@nxp.com>
> ---
>  drivers/i2c/busses/i2c-imx.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index d651ade86267..54fd5d0e4056 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -1892,9 +1892,15 @@ static void i2c_imx_remove(struct platform_device *pdev)
>  static int i2c_imx_runtime_suspend(struct device *dev)
>  {
>  	struct imx_i2c_struct *i2c_imx = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = pinctrl_pm_select_sleep_state(dev);
> +	if (ret)
> +		return ret;
>
>  	clk_disable(i2c_imx->clk);
> -	return pinctrl_pm_select_sleep_state(dev);
> +
> +	return 0;
>  }
>
>  static int i2c_imx_runtime_resume(struct device *dev)
> @@ -1907,10 +1913,13 @@ static int i2c_imx_runtime_resume(struct device *dev)
>  		return ret;
>
>  	ret = clk_enable(i2c_imx->clk);
> -	if (ret)
> +	if (ret) {
>  		dev_err(dev, "can't enable I2C clock, ret=%d\n", ret);
> +		pinctrl_pm_select_sleep_state(dev);
> +		return ret;
> +	}
>
> -	return ret;
> +	return 0;
>  }
>
>  static int __maybe_unused i2c_imx_suspend_noirq(struct device *dev)
> --
> 2.43.0
>