i2c: imx: Add timer for handling the stop condition
authorCorey Minyard <minyard@acm.org>
Fri, 12 Nov 2021 13:39:55 +0000 (07:39 -0600)
committerWolfram Sang <wsa@kernel.org>
Tue, 23 Nov 2021 10:16:56 +0000 (11:16 +0100)
Most IMX I2C interfaces don't generate an interrupt on a stop condition,
so it won't generate a timely stop event on a slave mode transfer.
Some users, like IPMB, need a timely stop event to work properly.

So, add a timer and add the proper handling to generate a stop event in
slave mode if the interface goes idle.

Signed-off-by: Corey Minyard <minyard@acm.org>
Tested-by: Andrew Manley <andrew.manley@sealingtech.com>
Reviewed-by: Andrew Manley <andrew.manley@sealingtech.com>
Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Wolfram Sang <wsa@kernel.org>
drivers/i2c/busses/i2c-imx.c

index 3576b63..27f969b 100644 (file)
@@ -37,6 +37,8 @@
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
+#include <linux/spinlock.h>
+#include <linux/hrtimer.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -51,6 +53,8 @@
 /* This will be the driver name the kernel reports */
 #define DRIVER_NAME "imx-i2c"
 
+#define I2C_IMX_CHECK_DELAY 30000 /* Time to check for bus idle, in NS */
+
 /*
  * Enable DMA if transfer byte size is bigger than this threshold.
  * As the hardware request, it must bigger than 4 bytes.\
@@ -210,6 +214,10 @@ struct imx_i2c_struct {
        struct imx_i2c_dma      *dma;
        struct i2c_client       *slave;
        enum i2c_slave_event last_slave_event;
+
+       /* For checking slave events. */
+       spinlock_t     slave_lock;
+       struct hrtimer slave_timer;
 };
 
 static const struct imx_i2c_hwdata imx1_i2c_hwdata = {
@@ -680,7 +688,7 @@ static void i2c_imx_slave_event(struct imx_i2c_struct *i2c_imx,
 
 static void i2c_imx_slave_finish_op(struct imx_i2c_struct *i2c_imx)
 {
-       u8 val;
+       u8 val = 0;
 
        while (i2c_imx->last_slave_event != I2C_SLAVE_STOP) {
                switch (i2c_imx->last_slave_event) {
@@ -701,10 +709,11 @@ static void i2c_imx_slave_finish_op(struct imx_i2c_struct *i2c_imx)
        }
 }
 
-static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
-                                    unsigned int status, unsigned int ctl)
+/* Returns true if the timer should be restarted, false if not. */
+static irqreturn_t i2c_imx_slave_handle(struct imx_i2c_struct *i2c_imx,
+                                       unsigned int status, unsigned int ctl)
 {
-       u8 value;
+       u8 value = 0;
 
        if (status & I2SR_IAL) { /* Arbitration lost */
                i2c_imx_clear_irq(i2c_imx, I2SR_IAL);
@@ -712,6 +721,16 @@ static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
                        return IRQ_HANDLED;
        }
 
+       if (!(status & I2SR_IBB)) {
+               /* No master on the bus, that could mean a stop condition. */
+               i2c_imx_slave_finish_op(i2c_imx);
+               return IRQ_HANDLED;
+       }
+
+       if (!(status & I2SR_ICF))
+               /* Data transfer still in progress, ignore this. */
+               goto out;
+
        if (status & I2SR_IAAS) { /* Addressed as a slave */
                i2c_imx_slave_finish_op(i2c_imx);
                if (status & I2SR_SRW) { /* Master wants to read from us*/
@@ -737,16 +756,9 @@ static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
                        imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
                }
        } else if (!(ctl & I2CR_MTX)) { /* Receive mode */
-               if (status & I2SR_IBB) { /* No STOP signal detected */
-                       value = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
-                       i2c_imx_slave_event(i2c_imx,
-                                           I2C_SLAVE_WRITE_RECEIVED, &value);
-               } else { /* STOP signal is detected */
-                       dev_dbg(&i2c_imx->adapter.dev,
-                               "STOP signal detected");
-                       i2c_imx_slave_event(i2c_imx,
-                                           I2C_SLAVE_STOP, &value);
-               }
+               value = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+               i2c_imx_slave_event(i2c_imx,
+                                   I2C_SLAVE_WRITE_RECEIVED, &value);
        } else if (!(status & I2SR_RXAK)) { /* Transmit mode received ACK */
                ctl |= I2CR_MTX;
                imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
@@ -755,15 +767,43 @@ static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
                                    I2C_SLAVE_READ_PROCESSED, &value);
 
                imx_i2c_write_reg(value, i2c_imx, IMX_I2C_I2DR);
-       } else { /* Transmit mode received NAK */
+       } else { /* Transmit mode received NAK, operation is done */
                ctl &= ~I2CR_MTX;
                imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
                imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+               i2c_imx_slave_finish_op(i2c_imx);
+               return IRQ_HANDLED;
        }
 
+out:
+       /*
+        * No need to check the return value here.  If it returns 0 or
+        * 1, then everything is fine.  If it returns -1, then the
+        * timer is running in the handler.  This will still work,
+        * though it may be redone (or already have been done) by the
+        * timer function.
+        */
+       hrtimer_try_to_cancel(&i2c_imx->slave_timer);
+       hrtimer_forward_now(&i2c_imx->slave_timer, I2C_IMX_CHECK_DELAY);
+       hrtimer_restart(&i2c_imx->slave_timer);
        return IRQ_HANDLED;
 }
 
+static enum hrtimer_restart i2c_imx_slave_timeout(struct hrtimer *t)
+{
+       struct imx_i2c_struct *i2c_imx = container_of(t, struct imx_i2c_struct,
+                                                     slave_timer);
+       unsigned int ctl, status;
+       unsigned long flags;
+
+       spin_lock_irqsave(&i2c_imx->slave_lock, flags);
+       status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
+       ctl = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+       i2c_imx_slave_handle(i2c_imx, status, ctl);
+       spin_unlock_irqrestore(&i2c_imx->slave_lock, flags);
+       return HRTIMER_NORESTART;
+}
+
 static void i2c_imx_slave_init(struct imx_i2c_struct *i2c_imx)
 {
        int temp;
@@ -843,7 +883,9 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
 {
        struct imx_i2c_struct *i2c_imx = dev_id;
        unsigned int ctl, status;
+       unsigned long flags;
 
+       spin_lock_irqsave(&i2c_imx->slave_lock, flags);
        status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
        ctl = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
 
@@ -851,14 +893,20 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
                i2c_imx_clear_irq(i2c_imx, I2SR_IIF);
                if (i2c_imx->slave) {
                        if (!(ctl & I2CR_MSTA)) {
-                               return i2c_imx_slave_isr(i2c_imx, status, ctl);
-                       } else if (i2c_imx->last_slave_event !=
-                                  I2C_SLAVE_STOP) {
-                               i2c_imx_slave_finish_op(i2c_imx);
+                               irqreturn_t ret;
+
+                               ret = i2c_imx_slave_handle(i2c_imx,
+                                                          status, ctl);
+                               spin_unlock_irqrestore(&i2c_imx->slave_lock,
+                                                      flags);
+                               return ret;
                        }
+                       i2c_imx_slave_finish_op(i2c_imx);
                }
+               spin_unlock_irqrestore(&i2c_imx->slave_lock, flags);
                return i2c_imx_master_isr(i2c_imx, status);
        }
+       spin_unlock_irqrestore(&i2c_imx->slave_lock, flags);
 
        return IRQ_NONE;
 }
@@ -1378,6 +1426,10 @@ static int i2c_imx_probe(struct platform_device *pdev)
        if (!i2c_imx)
                return -ENOMEM;
 
+       spin_lock_init(&i2c_imx->slave_lock);
+       hrtimer_init(&i2c_imx->slave_timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
+       i2c_imx->slave_timer.function = i2c_imx_slave_timeout;
+
        match = device_get_match_data(&pdev->dev);
        if (match)
                i2c_imx->hwdata = match;
@@ -1491,6 +1543,8 @@ static int i2c_imx_remove(struct platform_device *pdev)
        if (ret < 0)
                return ret;
 
+       hrtimer_cancel(&i2c_imx->slave_timer);
+
        /* remove adapter */
        dev_dbg(&i2c_imx->adapter.dev, "adapter removed\n");
        i2c_del_adapter(&i2c_imx->adapter);