• Jarkko Nikula's avatar
    i2c: designware: Fix slave state machine for sequential reads · c42edde5
    Jarkko Nikula authored
    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 3b5f7f10 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 3b5f7f10 ("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: default avatarJarkko Nikula <jarkko.nikula@linux.intel.com>
    Reviewed-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
    Signed-off-by: default avatarWolfram Sang <wsa@kernel.org>
    c42edde5
i2c-designware-core.h 12.5 KB