i2c: aspeed: Handle the coalesced stop conditions with the start conditions.

Some masters may drive the transfers with low enough latency between
the nak/stop phase of the current command and the start/address phase
of the following command that the interrupts are coalesced by the
time we process them.
Handle the stop conditions before processing SLAVE_MATCH to fix the
complaints that sometimes occur below.

"aspeed-i2c-bus 1e78a040.i2c-bus: irq handled != irq. Expected
0x00000086, but was 0x00000084"

Fixes: f9eb91350bb2 ("i2c: aspeed: added slave support for Aspeed I2C driver")
Signed-off-by: Quan Nguyen <[email protected]>
Reviewed-by: Andrew Jeffery <[email protected]>
Reviewed-by: Andi Shyti <[email protected]>
Signed-off-by: Wolfram Sang <[email protected]>

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 28e2a5fc45282df4e8b050efb9e1431752526d7d..5511fd46a65eae66b46f3e3385fe15f0c8970a2b 100644 (file)
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -249,18 +249,46 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
        if (!slave)
                return 0;
 
        if (!slave)
                return 0;
 

-       command = readl(bus->base + ASPEED_I2C_CMD_REG);
+       /*
+        * Handle stop conditions early, prior to SLAVE_MATCH. Some masters may drive
+        * transfers with low enough latency between the nak/stop phase of the current
+        * command and the start/address phase of the following command that the
+        * interrupts are coalesced by the time we process them.
+        */
+       if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
+               irq_handled |= ASPEED_I2CD_INTR_NORMAL_STOP;
+               bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+       }
+
+       if (irq_status & ASPEED_I2CD_INTR_TX_NAK &&
+           bus->slave_state == ASPEED_I2C_SLAVE_READ_PROCESSED) {
+               irq_handled |= ASPEED_I2CD_INTR_TX_NAK;
+               bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+       }
+
+       /* Propagate any stop conditions to the slave implementation. */
+       if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
+               i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
+               bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE;
+       }

 
 

-       /* Slave was requested, restart state machine. */
+       /*
+        * Now that we've dealt with any potentially coalesced stop conditions,
+        * address any start conditions.
+        */

        if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
                irq_handled |= ASPEED_I2CD_INTR_SLAVE_MATCH;
                bus->slave_state = ASPEED_I2C_SLAVE_START;
        }
 
        if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
                irq_handled |= ASPEED_I2CD_INTR_SLAVE_MATCH;
                bus->slave_state = ASPEED_I2C_SLAVE_START;
        }
 

-       /* Slave is not currently active, irq was for someone else. */
+       /*
+        * If the slave has been stopped and not started then slave interrupt
+        * handling is complete.
+        */

        if (bus->slave_state == ASPEED_I2C_SLAVE_INACTIVE)
                return irq_handled;
 
        if (bus->slave_state == ASPEED_I2C_SLAVE_INACTIVE)
                return irq_handled;
 

+       command = readl(bus->base + ASPEED_I2C_CMD_REG);

        dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
                irq_status, command);
 
        dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
                irq_status, command);
 


@@ -279,17 +307,6 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
                irq_handled |= ASPEED_I2CD_INTR_RX_DONE;
        }
 
                irq_handled |= ASPEED_I2CD_INTR_RX_DONE;
        }
 

-       /* Slave was asked to stop. */
-       if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
-               irq_handled |= ASPEED_I2CD_INTR_NORMAL_STOP;
-               bus->slave_state = ASPEED_I2C_SLAVE_STOP;
-       }
-       if (irq_status & ASPEED_I2CD_INTR_TX_NAK &&
-           bus->slave_state == ASPEED_I2C_SLAVE_READ_PROCESSED) {
-               irq_handled |= ASPEED_I2CD_INTR_TX_NAK;
-               bus->slave_state = ASPEED_I2C_SLAVE_STOP;
-       }
-

        switch (bus->slave_state) {
        case ASPEED_I2C_SLAVE_READ_REQUESTED:
                if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_ACK))
        switch (bus->slave_state) {
        case ASPEED_I2C_SLAVE_READ_REQUESTED:
                if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_ACK))


@@ -324,8 +341,7 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
                i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
                break;
        case ASPEED_I2C_SLAVE_STOP:
                i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
                break;
        case ASPEED_I2C_SLAVE_STOP:

-               i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
-               bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE;
+               /* Stop event handling is done early. Unreachable. */

                break;
        case ASPEED_I2C_SLAVE_START:
                /* Slave was just started. Waiting for the next event. */;
                break;
        case ASPEED_I2C_SLAVE_START:
                /* Slave was just started. Waiting for the next event. */;