Re: [PATCH v2] i2c: imx: Fix slave registration error path and missing timer cleanup

Hi Frank,

Thanks for the review. I'll split this into two separate patches and
send a v3 series.

Regards,
Liem

On Fri, Jun 26, 2026 at 12:16 AM Frank Li [off-list ref] wrote:

On Fri, Jun 26, 2026 at 12:02:19AM +0800, Liem wrote:

quoted

There are two issues that affect the i2c-imx slave handling:

1. In i2c_imx_reg_slave(), i2c_imx->slave is checked at the beginning
   and the function returns -EBUSY if it is non-NULL.  If
   pm_runtime_resume_and_get() fails later, the error path returns
   without clearing i2c_imx->slave, leaving it non-NULL.  Subsequent
   attempts to register a slave will then immediately fail with
   -EBUSY, making it impossible to register the slave again.  Fix
   by setting i2c_imx->slave = NULL on the error path.

2. In i2c_imx_unreg_slave(), the slave pointer is set to NULL after
   disabling interrupts.  However, a pending interrupt might already
   have started the hrtimer (i2c_imx_slave_timeout) before the pointer
   was cleared.  If the hrtimer fires after i2c_imx->slave is set to
   NULL, the timer callback i2c_imx_slave_finish_op() will call
   i2c_imx_slave_event() with a NULL slave pointer, and the
   last_slave_event check loop in i2c_imx_slave_finish_op() may cause
   a system hang because last_slave_event is no longer updated.  Fix
   by canceling the hrtimer and waiting for it to complete after
   disabling interrupts, before clearing the slave pointer.

Please use two patches to fix these problem. One patch fix one problem.

Frank

quoted

Both issues can trigger a kernel oops, system hang, or permanent
slave registration failure under certain race conditions.  Add the
missing NULL assignment and the missing hrtimer cleanup to harden
the slave path.

Fixes: f7414cd6923f ("i2c: imx: support slave mode for imx I2C driver")
Cc: stable@vger.kernel.org
Signed-off-by: Liem <redacted>
---
v1 -> v2:
  - Instead of adding a NULL check in i2c_imx_slave_event(), cancel
    the hrtimer and wait for it to finish in i2c_imx_unreg_slave()
    after disabling interrupts, as suggested by <Carlos Song>.
    This avoids a potential hang in the last_slave_event loop in
    i2c_imx_slave_finish_op().
---
 drivers/i2c/busses/i2c-imx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 28313d0fad37..04ffb927aba9 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c

@@ -936,6 +936,7 @@ static int i2c_imx_reg_slave(struct i2c_client *client)
        /* Resume */
        ret = pm_runtime_resume_and_get(i2c_imx->adapter.dev.parent);
        if (ret < 0) {
+               i2c_imx->slave = NULL;
                dev_err(&i2c_imx->adapter.dev, "failed to resume i2c controller");
                return ret;
        }

@@ -957,7 +958,7 @@ static int i2c_imx_unreg_slave(struct i2c_client *client)
        imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IADR);

        i2c_imx_reset_regs(i2c_imx);
-
+       hrtimer_cancel(&i2c_imx->slave_timer);
        i2c_imx->slave = NULL;

        /* Suspend */
--

2.53.0