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.wu@xxxxxxxxxx> --- 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