From a7ad8d09cdf0ec86612df0714d3e69ee92e6140b Mon Sep 17 00:00:00 2001 From: Jae Hyun Yoo Date: Tue, 20 Nov 2018 09:30:17 -0800 Subject: [PATCH] i2c: aspeed: Improve driver to support multi-master use cases stably In multi-master environment, this driver's master cannot know exactly when peer master sends data to this driver's slave so cases can be happened that this master tries to send data through the master_xfer function but slave data from a peer master is still being processed or slave xfer is started by a peer very after it queues a master command. To prevent state corruption in these cases, this patch adds the 'pending' state of master and its handling code so that the pending master xfer can be continued after slave mode session. Signed-off-by: Jae Hyun Yoo --- drivers/i2c/busses/i2c-aspeed.c | 119 ++++++++++++++++++++++++++++++---------- 1 file changed, 91 insertions(+), 28 deletions(-) diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c index 8dc9161ced38..d11b2ea97259 100644 --- a/drivers/i2c/busses/i2c-aspeed.c +++ b/drivers/i2c/busses/i2c-aspeed.c @@ -117,6 +117,7 @@ enum aspeed_i2c_master_state { ASPEED_I2C_MASTER_INACTIVE, + ASPEED_I2C_MASTER_PENDING, ASPEED_I2C_MASTER_START, ASPEED_I2C_MASTER_TX_FIRST, ASPEED_I2C_MASTER_TX, @@ -126,12 +127,13 @@ enum aspeed_i2c_master_state { }; enum aspeed_i2c_slave_state { - ASPEED_I2C_SLAVE_STOP, + ASPEED_I2C_SLAVE_INACTIVE, ASPEED_I2C_SLAVE_START, ASPEED_I2C_SLAVE_READ_REQUESTED, ASPEED_I2C_SLAVE_READ_PROCESSED, ASPEED_I2C_SLAVE_WRITE_REQUESTED, ASPEED_I2C_SLAVE_WRITE_RECEIVED, + ASPEED_I2C_SLAVE_STOP, }; struct aspeed_i2c_bus { @@ -156,6 +158,8 @@ struct aspeed_i2c_bus { int cmd_err; /* Protected only by i2c_lock_bus */ int master_xfer_result; + /* Multi-master */ + bool multi_master; #if IS_ENABLED(CONFIG_I2C_SLAVE) struct i2c_client *slave; enum aspeed_i2c_slave_state slave_state; @@ -251,7 +255,7 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status) } /* Slave is not currently active, irq was for someone else. */ - if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) + if (bus->slave_state == ASPEED_I2C_SLAVE_INACTIVE) return irq_handled; dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n", @@ -277,16 +281,15 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status) irq_handled |= ASPEED_I2CD_INTR_NORMAL_STOP; bus->slave_state = ASPEED_I2C_SLAVE_STOP; } - if (irq_status & ASPEED_I2CD_INTR_TX_NAK) { + 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; } - if (irq_status & ASPEED_I2CD_INTR_TX_ACK) - irq_handled |= ASPEED_I2CD_INTR_TX_ACK; switch (bus->slave_state) { case ASPEED_I2C_SLAVE_READ_REQUESTED: - if (irq_status & ASPEED_I2CD_INTR_TX_ACK) + if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_ACK)) dev_err(bus->dev, "Unexpected ACK on read request.\n"); bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED; i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value); @@ -294,9 +297,12 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status) writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG); break; case ASPEED_I2C_SLAVE_READ_PROCESSED: - if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK)) + if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) { dev_err(bus->dev, "Expected ACK after processed read.\n"); + break; + } + irq_handled |= ASPEED_I2CD_INTR_TX_ACK; i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value); writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG); writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG); @@ -310,10 +316,15 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status) break; case ASPEED_I2C_SLAVE_STOP: i2c_slave_event(slave, I2C_SLAVE_STOP, &value); + bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE; + break; + case ASPEED_I2C_SLAVE_START: + /* Slave was just started. Waiting for the next event. */; break; default: - dev_err(bus->dev, "unhandled slave_state: %d\n", + dev_err(bus->dev, "unknown slave_state: %d\n", bus->slave_state); + bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE; break; } @@ -328,7 +339,17 @@ static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus) struct i2c_msg *msg = &bus->msgs[bus->msgs_index]; u8 slave_addr = i2c_8bit_addr_from_msg(msg); - bus->master_state = ASPEED_I2C_MASTER_START; +#if IS_ENABLED(CONFIG_I2C_SLAVE) + /* + * If it's requested in the middle of a slave session, set the master + * state to 'pending' then H/W will continue handling this master + * command when the bus comes back to idle state. + */ + if (bus->slave_state != ASPEED_I2C_SLAVE_INACTIVE) + bus->master_state = ASPEED_I2C_MASTER_PENDING; + else +#endif /* CONFIG_I2C_SLAVE */ + bus->master_state = ASPEED_I2C_MASTER_START; bus->buf_index = 0; if (msg->flags & I2C_M_RD) { @@ -384,10 +405,6 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status) bus->master_state = ASPEED_I2C_MASTER_INACTIVE; irq_handled |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE; goto out_complete; - } else { - /* Master is not currently active, irq was for someone else. */ - if (bus->master_state == ASPEED_I2C_MASTER_INACTIVE) - goto out_no_complete; } /* @@ -399,12 +416,33 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status) if (ret) { dev_dbg(bus->dev, "received error interrupt: 0x%08x\n", irq_status); - bus->cmd_err = ret; - bus->master_state = ASPEED_I2C_MASTER_INACTIVE; irq_handled |= (irq_status & ASPEED_I2CD_INTR_MASTER_ERRORS); - goto out_complete; + if (bus->master_state != ASPEED_I2C_MASTER_INACTIVE) { + bus->cmd_err = ret; + bus->master_state = ASPEED_I2C_MASTER_INACTIVE; + goto out_complete; + } } +#if IS_ENABLED(CONFIG_I2C_SLAVE) + /* + * A pending master command will be started by H/W when the bus comes + * back to idle state after completing a slave operation so change the + * master state from 'pending' to 'start' at here if slave is inactive. + */ + if (bus->master_state == ASPEED_I2C_MASTER_PENDING) { + if (bus->slave_state != ASPEED_I2C_SLAVE_INACTIVE) + goto out_no_complete; + + bus->master_state = ASPEED_I2C_MASTER_START; + } +#endif /* CONFIG_I2C_SLAVE */ + + /* Master is not currently active, irq was for someone else. */ + if (bus->master_state == ASPEED_I2C_MASTER_INACTIVE || + bus->master_state == ASPEED_I2C_MASTER_PENDING) + goto out_no_complete; + /* We are in an invalid state; reset bus to a known state. */ if (!bus->msgs) { dev_err(bus->dev, "bus in unknown state. irq_status: 0x%x\n", @@ -423,6 +461,20 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status) * then update the state and handle the new state below. */ if (bus->master_state == ASPEED_I2C_MASTER_START) { +#if IS_ENABLED(CONFIG_I2C_SLAVE) + /* + * If a peer master starts a xfer very after it queues a master + * command, change its state to 'pending' then H/W will continue + * the queued master xfer just after completing the slave mode + * session. + */ + if (unlikely(irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH)) { + bus->master_state = ASPEED_I2C_MASTER_PENDING; + dev_dbg(bus->dev, + "master goes pending due to a slave start\n"); + goto out_no_complete; + } +#endif /* CONFIG_I2C_SLAVE */ if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) { if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_NAK))) { bus->cmd_err = -ENXIO; @@ -566,7 +618,8 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id) * interrupt bits. Each case needs to be handled using corresponding * handlers depending on the current state. */ - if (bus->master_state != ASPEED_I2C_MASTER_INACTIVE) { + if (bus->master_state != ASPEED_I2C_MASTER_INACTIVE && + bus->master_state != ASPEED_I2C_MASTER_PENDING) { irq_handled = aspeed_i2c_master_irq(bus, irq_remaining); irq_remaining &= ~irq_handled; if (irq_remaining) @@ -601,15 +654,14 @@ static int aspeed_i2c_master_xfer(struct i2c_adapter *adap, { struct aspeed_i2c_bus *bus = i2c_get_adapdata(adap); unsigned long time_left, flags; - int ret = 0; + int ret; spin_lock_irqsave(&bus->lock, flags); bus->cmd_err = 0; - /* If bus is busy, attempt recovery. We assume a single master - * environment. - */ - if (readl(bus->base + ASPEED_I2C_CMD_REG) & ASPEED_I2CD_BUS_BUSY_STS) { + /* If bus is busy in a single master environment, attempt recovery. */ + if (!bus->multi_master && + (readl(bus->base + ASPEED_I2C_CMD_REG) & ASPEED_I2CD_BUS_BUSY_STS)) { spin_unlock_irqrestore(&bus->lock, flags); ret = aspeed_i2c_recover_bus(bus); if (ret) @@ -629,10 +681,20 @@ static int aspeed_i2c_master_xfer(struct i2c_adapter *adap, time_left = wait_for_completion_timeout(&bus->cmd_complete, bus->adap.timeout); - if (time_left == 0) + if (time_left == 0) { + /* + * If timed out and bus is still busy in a multi master + * environment, attempt recovery at here. + */ + if (bus->multi_master && + (readl(bus->base + ASPEED_I2C_CMD_REG) & + ASPEED_I2CD_BUS_BUSY_STS)) + ret = aspeed_i2c_recover_bus(bus); + return -ETIMEDOUT; - else - return bus->master_xfer_result; + } + + return bus->master_xfer_result; } static u32 aspeed_i2c_functionality(struct i2c_adapter *adap) @@ -672,7 +734,7 @@ static int aspeed_i2c_reg_slave(struct i2c_client *client) __aspeed_i2c_reg_slave(bus, client->addr); bus->slave = client; - bus->slave_state = ASPEED_I2C_SLAVE_STOP; + bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE; spin_unlock_irqrestore(&bus->lock, flags); return 0; @@ -827,7 +889,9 @@ static int aspeed_i2c_init(struct aspeed_i2c_bus *bus, if (ret < 0) return ret; - if (!of_property_read_bool(pdev->dev.of_node, "multi-master")) + if (of_property_read_bool(pdev->dev.of_node, "multi-master")) + bus->multi_master = true; + else fun_ctrl_reg |= ASPEED_I2CD_MULTI_MASTER_DIS; /* Enable Master Mode */ @@ -930,7 +994,6 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev) init_completion(&bus->cmd_complete); bus->adap.owner = THIS_MODULE; bus->adap.retries = 0; - bus->adap.timeout = 5 * HZ; bus->adap.algo = &aspeed_i2c_algo; bus->adap.dev.parent = &pdev->dev; bus->adap.dev.of_node = pdev->dev.of_node; -- 2.7.4