RE: [PATCH] i2c: imx-lpi2c: avoid accessing target registers on master-only controllers
From: Carlos Song
Date: Wed Aug 26 2026 - 04:00:32 EST
NXP Public
> -----Original Message-----
> From: Carlos Song (OSS)
> Sent: Wednesday, August 26, 2026 11:24 AM
> To: Frank Li (OSS) <frank.li@xxxxxxxxxxx>; andi.shyti@xxxxxxxxxx;
> kernel@xxxxxxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; Frank Li
> <frank.li@xxxxxxx>; Aisheng Dong <aisheng.dong@xxxxxxx>;
> festevam@xxxxxxxxx
> Cc: linux-i2c@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH] i2c: imx-lpi2c: avoid accessing target registers on
> master-only controllers
>
> Gentle ping to avoid missing in mail sea.
>
> Thanks.
>
> > -----Original Message-----
> > From: Frank Li (OSS) <frank.li@xxxxxxxxxxx>
> > Sent: Saturday, August 8, 2026 3:25 AM
> > To: Carlos Song (OSS) <carlos.song@xxxxxxxxxxx>
> > Cc: Aisheng Dong <aisheng.dong@xxxxxxx>; andi.shyti@xxxxxxxxxx; Frank
> > Li <frank.li@xxxxxxx>; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx;
> > festevam@xxxxxxxxx; Carlos Song <carlos.song@xxxxxxx>;
> > linux-i2c@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx;
> > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH] i2c: imx-lpi2c: avoid accessing target registers
> > on master-only controllers
> >
> > On Mon, Aug 03, 2026 at 11:27:05AM +0800, carlos.song@xxxxxxxxxxx
> wrote:
> > > From: Carlos Song <carlos.song@xxxxxxx>
> > >
> > > 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@xxxxxxx>
> > > ---
> >
> > Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
> >
> > > 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
> > >
> > >