@@ -103,7 +103,6 @@
#define DW_IC_INTR_MASTER_MASK (DW_IC_INTR_DEFAULT_MASK | \
DW_IC_INTR_TX_EMPTY)
#define DW_IC_INTR_SLAVE_MASK (DW_IC_INTR_DEFAULT_MASK | \
- DW_IC_INTR_RX_DONE | \
DW_IC_INTR_RX_UNDER | \
DW_IC_INTR_RD_REQ)
@@ -173,8 +173,9 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
enabled, slave_activity, raw_stat, stat);
if (stat & DW_IC_INTR_RX_FULL) {
- if (dev->status != STATUS_WRITE_IN_PROGRESS) {
- dev->status = STATUS_WRITE_IN_PROGRESS;
+ if (!(dev->status & STATUS_WRITE_IN_PROGRESS)) {
+ dev->status |= STATUS_WRITE_IN_PROGRESS;
+ dev->status &= ~STATUS_READ_IN_PROGRESS;
i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED,
&val);
}
@@ -190,24 +191,23 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
if (slave_activity) {
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
- dev->status = STATUS_READ_IN_PROGRESS;
- if (!i2c_slave_event(dev->slave,
- I2C_SLAVE_READ_REQUESTED,
- &val))
- regmap_write(dev->map, DW_IC_DATA_CMD, val);
+ if (!(dev->status & STATUS_READ_IN_PROGRESS)) {
+ i2c_slave_event(dev->slave,
+ I2C_SLAVE_READ_REQUESTED,
+ &val);
+ dev->status |= STATUS_READ_IN_PROGRESS;
+ dev->status &= ~STATUS_WRITE_IN_PROGRESS;
+ } else {
+ i2c_slave_event(dev->slave,
+ I2C_SLAVE_READ_PROCESSED,
+ &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);
- }
-
- if (stat & DW_IC_INTR_STOP_DET) {
- dev->status = STATUS_IDLE;
+ if (stat & DW_IC_INTR_STOP_DET)
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
- }
return 1;
}
Some read types from I2C bus don't work correctly when testing the i2c-designware-slave.c with the slave-eeprom backend. The same reads work correctly when testing with a real 24c02 EEPROM chip. In the following tests an i2c-designware-slave.c instance with the slave-eeprom backend is configured to act as a simulated 24c02 at address 0x65 on an I2C host bus 6: 1. i2cdump -y 6 0x65 b (OK) Random read. Each byte are read using a byte address write with a current address read in a same message. 2. i2cdump -y 6 0x65 c (OK, was NOK before commit 3b5f7f10ff6e when it was repeating the 1st byte) Repeated current address read. One byte address write message followed by repeated current address read messages. 3. i2cdump -y 6 0x65 i (NOK, each 32 byte block repeats the 1st byte of block) Sequential read using SMBus Block Read. For each 32 byte block a byte address write followed by 32 sequental reads in a same message. These findings are explained because the implementation has had a mismatch between hardware interrupts and what I2C slave events should be sent after those interrupts. Despite that the case 1 happened to have always the I2C slave events sent to a right order with a right data between backend and the I2C bus. Hardware generates the DW_IC_INTR_RD_REQ interrupt when another host is attempting to read and for sequential reads after. DW_IC_INTR_RX_DONE occurs when host does not acknowledge a transmitted byte which is an indication the end of transmission. Those interrupts do not match directly with I2C_SLAVE_READ_REQUESTED and I2C_SLAVE_READ_PROCESSED events which is how the code was and is practically using them. The slave-eeprom backend increases the buffer index with the I2C_SLAVE_READ_PROCESSED event and returns the data from current index when receiving only the I2C_SLAVE_READ_REQUESTED event. That explains the repeated bytes in case 3 and also case 2 before commit 3b5f7f10ff6e ("i2c: designware: slave should do WRITE_REQUESTED before WRITE_RECEIVED"). Patch fixes the case 3 while keep cases 1 and 2 working with following changes: - First DW_IC_INTR_RD_REQ interrupt will change the state machine to read in progress state, send I2C_SLAVE_READ_REQUESTED event and transmit the first byte from backend - Subsequent DW_IC_INTR_RD_REQ interrupts will send I2C_SLAVE_READ_PROCESSED events and transmit next bytes from backend - STOP won't change the state machine. Otherwise case 2 won't work since we cannot distinguish current address read from sequentiel read - DW_IC_INTR_RX_DONE interrupt is needless since there is no mechanism to inform it to a backend. It cannot be used to change state machine at the end of read either due the same reason than above - Next host write to us will change the state machine from read to write in progress state - STATUS_WRITE_IN_PROGRESS and STATUS_READ_IN_PROGRESS are considered now to be status flags not the state of the driver. This is how we treat them in i2c-designware-master.c While at it do not test the return code from i2c_slave_event() for I2C_SLAVE_READ_REQUESTED and I2C_SLAVE_READ_PROCESSED since it returns always 0. Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/i2c/busses/i2c-designware-core.h | 1 - drivers/i2c/busses/i2c-designware-slave.c | 32 +++++++++++------------ 2 files changed, 16 insertions(+), 17 deletions(-)