Sometime we would get the following flows when doing an i2cset:

1. No any WRITE_REQUESTED
0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
WRITE_RECEIVED
0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
WRITE_RECEIVED
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x710 : INTR_STAT=0x200
STOP

2. WRITE_REQUESTED after WRITE_RECEIVED
0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
WRITE_RECEIVED
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : INTR_STAT=0x204
WRITE_RECEIVED
WRITE_REQUESTED

Documentation/i2c/slave-interface.rst said that I2C_SLAVE_WRITE_REQUESTED,
which is mandatory, comes without any data arrived. It means
I2C_SLAVE_WRITE_REQUESTED should be in front of any
I2C_SLAVE_WRITE_RECEIVED in a write-request. Obviously the 1st log shows
no I2C_SLAVE_WRITE_REQUESTED, and 2nd log shows I2C_SLAVE_WRITE_REQUESTED
occurred after I2C_SLAVE_WRITE_RECEIVED.

I2C_SLAVE_STOP is mandatory to notify a request finish to reset the state
machine of the backend. In case 2 it never do I2C_SLAVE_STOP when STOP
condition is received. This is one of illegal issues to be fixed.

dev->status can be used to record the current machine state, especially
DesignWare I2C controller has no interrupts to notify a new write-request
coming, an IC_INTR_RX_FULL rising while dev->status isn't
STATUS_WRITE_IN_PROGRESS is notified to do an I2C_SLAVE_WRITE_REQUESTED
by checking dev->status. dev->status also helps to resolve the omitted
I2C_SLAVE_STOP in case 2.

Signed-off-by: Michael Wu <michael...@vatics.com>
---
 drivers/i2c/busses/i2c-designware-slave.c | 73 ++++++++++++-----------
 1 file changed, 39 insertions(+), 34 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-slave.c 
b/drivers/i2c/busses/i2c-designware-slave.c
index 02e7c5171827..57ce1f0b403c 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -172,50 +172,55 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev 
*dev)
                "%#x STATUS SLAVE_ACTIVITY=%#x : RAW_INTR_STAT=%#x : 
INTR_STAT=%#x\n",
                enabled, slave_activity, raw_stat, stat);
 
+       if (stat & DW_IC_INTR_RX_FULL) {
+               if (dev->status != STATUS_WRITE_IN_PROGRESS) {
+                       if (dev->status != STATUS_IDLE)
+                               i2c_slave_event(dev->slave, I2C_SLAVE_STOP,
+                                               &val);
+
+                       dev->status = STATUS_WRITE_IN_PROGRESS;
+                       i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED,
+                                       &val);
+               }
+
+               regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+               val = tmp;
+               if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
+                                    &val))
+                       dev_vdbg(dev->dev, "Byte %X acked!", val);
+       }
+
        if (stat & DW_IC_INTR_RD_REQ) {
+               if (dev->status != STATUS_IDLE) {
+                       dev->status = STATUS_IDLE;
+                       i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
+               }
+
                if (slave_activity) {
-                       if (stat & DW_IC_INTR_RX_FULL) {
-                               regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
-                               val = tmp;
-
-                               if (!i2c_slave_event(dev->slave,
-                                                    I2C_SLAVE_WRITE_RECEIVED,
-                                                    &val)) {
-                                       dev_vdbg(dev->dev, "Byte %X acked!",
-                                                val);
-                               }
-                               regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
-                       } else {
-                               regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
-                               regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
-                       }
-                       if (!i2c_slave_event(dev->slave,
-                                            I2C_SLAVE_READ_REQUESTED,
-                                            &val))
-                               regmap_write(dev->map, DW_IC_DATA_CMD, val);
+                       regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
+                       regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
+
+                       dev->status = STATUS_READ_IN_PROGRESS;
+                       i2c_slave_event(dev->slave, I2C_SLAVE_READ_REQUESTED,
+                                       &val);
+                       regmap_write(dev->map, DW_IC_DATA_CMD, val);
                }
        }
 
        if (stat & DW_IC_INTR_RX_DONE) {
-               if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED,
-                                    &val))
-                       regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
+               i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED, &val);
+               regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
 
+               dev->status = STATUS_IDLE;
                i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-               return 1;
        }
 
-       if (stat & DW_IC_INTR_RX_FULL) {
-               regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
-               val = tmp;
-               if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
-                                    &val))
-                       dev_vdbg(dev->dev, "Byte %X acked!", val);
-       } else
-               i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-
-       if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
-               i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
+       if (stat & DW_IC_INTR_STOP_DET) {
+               if (dev->status != STATUS_IDLE) {
+                       dev->status = STATUS_IDLE;
+                       i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
+               }
+       }
 
        return 1;
 }
-- 
2.17.1

Reply via email to