[PATCH] i2c: imx-lpi2c: avoid accessing target registers on master-only controllers

[PATCH] i2c: imx-lpi2c: avoid accessing target registers on master-only controllers

[carlos.song]

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

Not all LPI2C controller instances implement the Target block.
Since commit 90311787f483 ("i2c: imx-lpi2c: reset controller in
probe stage"), the driver unconditionally resets both the Master
and Target blocks during probe.

On controllers that do not support target mode, accessing the
Target registers triggers an asynchronous SError and prevents the
driver from probing successfully. For example on i.MX8QM:

  SError Interrupt on CPU2, code 0x00000000bf000002 -- SError
  Hardware name: Freescale i.MX8QM MEK (DT)
  pc : lpi2c_imx_probe+0x280/0x594
  lr : lpi2c_imx_probe+0x224/0x594
  Kernel panic - not syncing: Asynchronous SError Interrupt

The VERID register is implemented in the Master block and can be
safely accessed on all controller variants. Its FEATURE field
indicates whether target mode is supported.

Read VERID during probe and use it to determine whether the
Target block is present. Only access Target registers when target
mode is supported and reject target registration requests with
-EOPNOTSUPP otherwise.

Fixes: 90311787f483 ("i2c: imx-lpi2c: reset controller in probe stage")
Signed-off-by: Carlos Song <carlos.song@nxp.com>
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index e1a4338bc51e..1cfd7a4c8237 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -29,6 +29,7 @@
 
 #define DRIVER_NAME "imx-lpi2c"
 
+#define LPI2C_VERID	0x00	/* i2c version ID */
 #define LPI2C_PARAM	0x04	/* i2c RX/TX FIFO size */
 #define LPI2C_MCR	0x10	/* i2c contrl register */
 #define LPI2C_MSR	0x14	/* i2c status register */
@@ -136,6 +137,9 @@
 #define I2C_PM_LONG_TIMEOUT_MS	1000 /* Avoid dead lock caused by big clock prepare lock */
 #define I2C_DMA_THRESHOLD	8 /* bytes */
 
+/* Bit 0 indicates the presence of the target feature */
+#define VERID_FEATURE_TARGET_PRESENT	BIT(0)
+
 enum lpi2c_imx_mode {
 	STANDARD,	/* 100+Kbps */
 	FAST,		/* 400+Kbps */
@@ -194,6 +198,7 @@ struct lpi2c_imx_struct {
 	bool			can_use_dma;
 	struct lpi2c_imx_dma	*dma;
 	struct i2c_client	*target;
+	bool			target_supported;
 	int			irq;
 	const struct imx_lpi2c_hwdata *hwdata;
 };
@@ -1330,6 +1335,10 @@ static int lpi2c_imx_register_target(struct i2c_client *client)
 	struct lpi2c_imx_struct *lpi2c_imx = i2c_get_adapdata(client->adapter);
 	int ret;
 
+	/* Reject target-mode registration on controllers that don't support it. */
+	if (!lpi2c_imx->target_supported)
+		return -EOPNOTSUPP;
+
 	if (lpi2c_imx->target)
 		return -EBUSY;
 
@@ -1546,13 +1555,23 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 
 	/*
-	 * Reset all internal controller registers of both Master and Target
-	 * to avoid effects of previous status.
+	 * Reset all internal controller registers to avoid effects of any
+	 * state left over from a previous stage (e.g. the bootloader).
+	 *
+	 * The Master block (MCR) is present on every controller, so reset it
+	 * unconditionally. VERID shows whether the target feature is supported.
+	 * Do not touch the Target block (SCR) on a master-only controller to
+	 * avoid an asynchronous SError.
 	 */
 	writel(MCR_RST, lpi2c_imx->base + LPI2C_MCR);
-	writel(SCR_RST, lpi2c_imx->base + LPI2C_SCR);
 	writel(0, lpi2c_imx->base + LPI2C_MCR);
-	writel(0, lpi2c_imx->base + LPI2C_SCR);
+
+	lpi2c_imx->target_supported = !!(readl(lpi2c_imx->base + LPI2C_VERID) &
+					 VERID_FEATURE_TARGET_PRESENT);
+	if (lpi2c_imx->target_supported) {
+		writel(SCR_RST, lpi2c_imx->base + LPI2C_SCR);
+		writel(0, lpi2c_imx->base + LPI2C_SCR);
+	}
 
 	ret = devm_request_irq(&pdev->dev, lpi2c_imx->irq, lpi2c_imx_isr, IRQF_NO_SUSPEND,
 			       pdev->name, lpi2c_imx);
-- 
2.43.0

Re: [PATCH] i2c: imx-lpi2c: avoid accessing target registers on master-only controllers

[Frank Li]

On Mon, Aug 03, 2026 at 11:27:05AM +0800, carlos.song@oss.nxp.com wrote:
> From: Carlos Song <carlos.song@nxp.com>
>
> Not all LPI2C controller instances implement the Target block.
> Since commit 90311787f483 ("i2c: imx-lpi2c: reset controller in
> probe stage"), the driver unconditionally resets both the Master
> and Target blocks during probe.
>
> On controllers that do not support target mode, accessing the
> Target registers triggers an asynchronous SError and prevents the
> driver from probing successfully. For example on i.MX8QM:
>
>   SError Interrupt on CPU2, code 0x00000000bf000002 -- SError
>   Hardware name: Freescale i.MX8QM MEK (DT)
>   pc : lpi2c_imx_probe+0x280/0x594
>   lr : lpi2c_imx_probe+0x224/0x594
>   Kernel panic - not syncing: Asynchronous SError Interrupt
>
> The VERID register is implemented in the Master block and can be
> safely accessed on all controller variants. Its FEATURE field
> indicates whether target mode is supported.
>
> Read VERID during probe and use it to determine whether the
> Target block is present. Only access Target registers when target
> mode is supported and reject target registration requests with
> -EOPNOTSUPP otherwise.
>
> Fixes: 90311787f483 ("i2c: imx-lpi2c: reset controller in probe stage")
> Signed-off-by: Carlos Song <carlos.song@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/i2c/busses/i2c-imx-lpi2c.c | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index e1a4338bc51e..1cfd7a4c8237 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -29,6 +29,7 @@
>
>  #define DRIVER_NAME "imx-lpi2c"
>
> +#define LPI2C_VERID	0x00	/* i2c version ID */
>  #define LPI2C_PARAM	0x04	/* i2c RX/TX FIFO size */
>  #define LPI2C_MCR	0x10	/* i2c contrl register */
>  #define LPI2C_MSR	0x14	/* i2c status register */
> @@ -136,6 +137,9 @@
>  #define I2C_PM_LONG_TIMEOUT_MS	1000 /* Avoid dead lock caused by big clock prepare lock */
>  #define I2C_DMA_THRESHOLD	8 /* bytes */
>
> +/* Bit 0 indicates the presence of the target feature */
> +#define VERID_FEATURE_TARGET_PRESENT	BIT(0)
> +
>  enum lpi2c_imx_mode {
>  	STANDARD,	/* 100+Kbps */
>  	FAST,		/* 400+Kbps */
> @@ -194,6 +198,7 @@ struct lpi2c_imx_struct {
>  	bool			can_use_dma;
>  	struct lpi2c_imx_dma	*dma;
>  	struct i2c_client	*target;
> +	bool			target_supported;
>  	int			irq;
>  	const struct imx_lpi2c_hwdata *hwdata;
>  };
> @@ -1330,6 +1335,10 @@ static int lpi2c_imx_register_target(struct i2c_client *client)
>  	struct lpi2c_imx_struct *lpi2c_imx = i2c_get_adapdata(client->adapter);
>  	int ret;
>
> +	/* Reject target-mode registration on controllers that don't support it. */
> +	if (!lpi2c_imx->target_supported)
> +		return -EOPNOTSUPP;
> +
>  	if (lpi2c_imx->target)
>  		return -EBUSY;
>
> @@ -1546,13 +1555,23 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
>  	pm_runtime_enable(&pdev->dev);
>
>  	/*
> -	 * Reset all internal controller registers of both Master and Target
> -	 * to avoid effects of previous status.
> +	 * Reset all internal controller registers to avoid effects of any
> +	 * state left over from a previous stage (e.g. the bootloader).
> +	 *
> +	 * The Master block (MCR) is present on every controller, so reset it
> +	 * unconditionally. VERID shows whether the target feature is supported.
> +	 * Do not touch the Target block (SCR) on a master-only controller to
> +	 * avoid an asynchronous SError.
>  	 */
>  	writel(MCR_RST, lpi2c_imx->base + LPI2C_MCR);
> -	writel(SCR_RST, lpi2c_imx->base + LPI2C_SCR);
>  	writel(0, lpi2c_imx->base + LPI2C_MCR);
> -	writel(0, lpi2c_imx->base + LPI2C_SCR);
> +
> +	lpi2c_imx->target_supported = !!(readl(lpi2c_imx->base + LPI2C_VERID) &
> +					 VERID_FEATURE_TARGET_PRESENT);
> +	if (lpi2c_imx->target_supported) {
> +		writel(SCR_RST, lpi2c_imx->base + LPI2C_SCR);
> +		writel(0, lpi2c_imx->base + LPI2C_SCR);
> +	}
>
>  	ret = devm_request_irq(&pdev->dev, lpi2c_imx->irq, lpi2c_imx_isr, IRQF_NO_SUSPEND,
>  			       pdev->name, lpi2c_imx);
> --
> 2.43.0
>
>