lkml.org 
[lkml]   [2016]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v5 03/14] i2c: octeon: Improve error handling
    Date
    From: Peter Swain <pswain@cavium.com>

    Consider more status codes and improve error handling.
    Distinguish handling for first and last part of a message.

    TODO: Convert to use the i2c recovery framework.

    Signed-off-by: Peter Swain <pswain@cavium.com>
    Signed-off-by: Jan Glauber <jglauber@cavium.com>
    ---
    drivers/i2c/busses/i2c-octeon.c | 244 +++++++++++++++++++++++++++++-----------
    1 file changed, 178 insertions(+), 66 deletions(-)

    diff --git a/drivers/i2c/busses/i2c-octeon.c b/drivers/i2c/busses/i2c-octeon.c
    index f647667..a037245 100644
    --- a/drivers/i2c/busses/i2c-octeon.c
    +++ b/drivers/i2c/busses/i2c-octeon.c
    @@ -56,12 +56,34 @@
    #define TWSI_CTL_AAK 0x04 /* Assert ACK */

    /* Some status values */
    +#define STAT_ERROR 0x00
    #define STAT_START 0x08
    #define STAT_RSTART 0x10
    #define STAT_TXADDR_ACK 0x18
    +#define STAT_TXADDR_NAK 0x20
    #define STAT_TXDATA_ACK 0x28
    +#define STAT_TXDATA_NAK 0x30
    +#define STAT_LOST_ARB_38 0x38
    #define STAT_RXADDR_ACK 0x40
    +#define STAT_RXADDR_NAK 0x48
    #define STAT_RXDATA_ACK 0x50
    +#define STAT_RXDATA_NAK 0x58
    +#define STAT_SLAVE_60 0x60
    +#define STAT_LOST_ARB_68 0x68
    +#define STAT_SLAVE_70 0x70
    +#define STAT_LOST_ARB_78 0x78
    +#define STAT_SLAVE_80 0x80
    +#define STAT_SLAVE_88 0x88
    +#define STAT_GENDATA_ACK 0x90
    +#define STAT_GENDATA_NAK 0x98
    +#define STAT_SLAVE_A0 0xA0
    +#define STAT_SLAVE_A8 0xA8
    +#define STAT_LOST_ARB_B0 0xB0
    +#define STAT_SLAVE_LOST 0xB8
    +#define STAT_SLAVE_NAK 0xC0
    +#define STAT_SLAVE_ACK 0xC8
    +#define STAT_AD2W_ACK 0xD0
    +#define STAT_AD2W_NAK 0xD8
    #define STAT_IDLE 0xF8

    /* TWSI_INT values */
    @@ -79,6 +101,8 @@ struct octeon_i2c {
    struct device *dev;
    };

    +static int reset_how;
    +
    /**
    * octeon_i2c_write_sw - write an I2C core register
    * @i2c: The struct octeon_i2c
    @@ -186,7 +210,6 @@ static irqreturn_t octeon_i2c_isr(int irq, void *dev_id)
    return IRQ_HANDLED;
    }

    -
    static int octeon_i2c_test_iflg(struct octeon_i2c *i2c)
    {
    return (octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_CTL) & TWSI_CTL_IFLG) != 0;
    @@ -214,6 +237,66 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c)
    return 0;
    }

    +static int octeon_i2c_lost_arb(u8 code, int final_read)
    +{
    + switch (code) {
    + /* Arbitration lost */
    + case STAT_LOST_ARB_38:
    + case STAT_LOST_ARB_68:
    + case STAT_LOST_ARB_78:
    + case STAT_LOST_ARB_B0:
    + return -EAGAIN;
    +
    + /* Being addressed as slave, should back off & listen */
    + case STAT_SLAVE_60:
    + case STAT_SLAVE_70:
    + case STAT_GENDATA_ACK:
    + case STAT_GENDATA_NAK:
    + return -EIO;
    +
    + /* Core busy as slave */
    + case STAT_SLAVE_80:
    + case STAT_SLAVE_88:
    + case STAT_SLAVE_A0:
    + case STAT_SLAVE_A8:
    + case STAT_SLAVE_LOST:
    + case STAT_SLAVE_NAK:
    + case STAT_SLAVE_ACK:
    + return -EIO;
    +
    + /* ACK allowed on pre-terminal bytes only */
    + case STAT_RXDATA_ACK:
    + if (!final_read)
    + return 0;
    + return -EAGAIN;
    +
    + /* NAK allowed on terminal byte only */
    + case STAT_RXDATA_NAK:
    + if (final_read)
    + return 0;
    + return -EAGAIN;
    + case STAT_TXDATA_NAK:
    + case STAT_TXADDR_NAK:
    + case STAT_RXADDR_NAK:
    + case STAT_AD2W_NAK:
    + return -EAGAIN;
    + }
    + return 0;
    +}
    +
    +static int check_arb(struct octeon_i2c *i2c, int final_read)
    +{
    + return octeon_i2c_lost_arb(octeon_i2c_read_sw(i2c,
    + SW_TWSI_EOP_TWSI_STAT), final_read);
    +}
    +
    +/* send STOP to the bus */
    +static void octeon_i2c_stop(struct octeon_i2c *i2c)
    +{
    + octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
    + TWSI_CTL_ENAB | TWSI_CTL_STP);
    +}
    +
    /* calculate and set clock divisors */
    static void octeon_i2c_set_clock(struct octeon_i2c *i2c)
    {
    @@ -277,71 +360,103 @@ static int octeon_i2c_init_lowlevel(struct octeon_i2c *i2c)
    return -EIO;
    }

    +/*
    + * TWSI state seems stuck. Not sure if it's TWSI-engine state or something
    + * else on bus. The initial _stop() is always harmless, it just resets state
    + * machine, does not _transmit_ STOP unless engine was active.
    + */
    +static int start_unstick(struct octeon_i2c *i2c)
    +{
    + octeon_i2c_stop(i2c);
    +
    + /*
    + * Response is escalated over successive calls,
    + * as EAGAIN provokes retries from i2c/core.
    + */
    + switch (reset_how++ % 4) {
    + case 0:
    + /* just the stop above */
    + break;
    + case 1:
    + /*
    + * Controller refused to send start flag. May be a
    + * client is holding SDA low? Let's try to free it.
    + */
    + octeon_i2c_unblock(i2c);
    + break;
    + case 2:
    + /* re-init our TWSI hardware */
    + octeon_i2c_init_lowlevel(i2c);
    + break;
    + default:
    + /* retry in caller */
    + reset_how = 0;
    + return -EAGAIN;
    + }
    + return 0;
    +}
    +
    /**
    * octeon_i2c_start - send START to the bus
    * @i2c: The struct octeon_i2c
    + * @first: first msg in combined operation?
    *
    * Returns 0 on success, otherwise a negative errno.
    */
    -static int octeon_i2c_start(struct octeon_i2c *i2c)
    +static int octeon_i2c_start(struct octeon_i2c *i2c, int first)
    {
    int result;
    u8 data;

    - octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
    - TWSI_CTL_ENAB | TWSI_CTL_STA);
    + while (1) {
    + octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
    + TWSI_CTL_ENAB | TWSI_CTL_STA);

    - result = octeon_i2c_wait(i2c);
    - if (result) {
    - if (octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_STAT) == STAT_IDLE) {
    - /*
    - * Controller refused to send start flag May
    - * be a client is holding SDA low - let's try
    - * to free it.
    - */
    - octeon_i2c_unblock(i2c);
    - octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
    - TWSI_CTL_ENAB | TWSI_CTL_STA);
    - result = octeon_i2c_wait(i2c);
    + result = octeon_i2c_wait(i2c);
    + data = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_STAT);
    +
    + switch (data) {
    + case STAT_START:
    + case STAT_RSTART:
    + if (!first)
    + return -EAGAIN;
    + reset_how = 0;
    + return 0;
    + case STAT_RXADDR_ACK:
    + if (first)
    + return -EAGAIN;
    + return start_unstick(i2c);
    + /*
    + * case STAT_IDLE:
    + * case STAT_ERROR:
    + */
    + default:
    + if (!first)
    + return -EAGAIN;
    + start_unstick(i2c);
    }
    - if (result)
    - return result;
    }
    -
    - data = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_STAT);
    - if ((data != STAT_START) && (data != STAT_RSTART)) {
    - dev_err(i2c->dev, "%s: bad status (0x%x)\n", __func__, data);
    - return -EIO;
    - }
    -
    return 0;
    }

    -/* send STOP to the bus */
    -static void octeon_i2c_stop(struct octeon_i2c *i2c)
    -{
    - octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
    - TWSI_CTL_ENAB | TWSI_CTL_STP);
    -}
    -
    /**
    * octeon_i2c_write - send data to the bus via low-level controller
    * @i2c: The struct octeon_i2c
    * @target: Target address
    * @data: Pointer to the data to be sent
    * @length: Length of the data
    + * @last: is last msg in combined operation?
    *
    * The address is sent over the bus, then the data.
    *
    * Returns 0 on success, otherwise a negative errno.
    */
    static int octeon_i2c_write(struct octeon_i2c *i2c, int target,
    - const u8 *data, int length)
    + const u8 *data, int length, int first, int last)
    {
    int i, result;
    - u8 tmp;

    - result = octeon_i2c_start(i2c);
    + result = octeon_i2c_start(i2c, first);
    if (result)
    return result;

    @@ -353,14 +468,9 @@ static int octeon_i2c_write(struct octeon_i2c *i2c, int target,
    return result;

    for (i = 0; i < length; i++) {
    - tmp = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_STAT);
    -
    - if ((tmp != STAT_TXADDR_ACK) && (tmp != STAT_TXDATA_ACK)) {
    - dev_err(i2c->dev,
    - "%s: bad status before write (0x%x)\n",
    - __func__, tmp);
    - return -EIO;
    - }
    + result = check_arb(i2c, false);
    + if (result)
    + return result;

    octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_DATA, data[i]);
    octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL, TWSI_CTL_ENAB);
    @@ -368,6 +478,9 @@ static int octeon_i2c_write(struct octeon_i2c *i2c, int target,
    result = octeon_i2c_wait(i2c);
    if (result)
    return result;
    + result = check_arb(i2c, false);
    + if (result)
    + return result;
    }

    return 0;
    @@ -379,54 +492,51 @@ static int octeon_i2c_write(struct octeon_i2c *i2c, int target,
    * @target: Target address
    * @data: Pointer to the location to store the data
    * @rlength: Length of the data
    + * @phase: which phase of a combined operation.
    * @recv_len: flag for length byte
    *
    * The address is sent over the bus, then the data is read.
    *
    * Returns 0 on success, otherwise a negative errno.
    */
    -static int octeon_i2c_read(struct octeon_i2c *i2c, int target,
    - u8 *data, u16 *rlength, bool recv_len)
    +static int octeon_i2c_read(struct octeon_i2c *i2c, int target, u8 *data,
    + u16 *rlength, bool first, bool last, bool recv_len)
    {
    + u8 ctl = TWSI_CTL_ENAB | TWSI_CTL_AAK;
    int i, result, length = *rlength;
    u8 tmp;

    if (length < 1)
    return -EINVAL;

    - result = octeon_i2c_start(i2c);
    + result = octeon_i2c_start(i2c, first);
    if (result)
    return result;

    octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_DATA, (target << 1) | 1);
    - octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL, TWSI_CTL_ENAB);
    -
    - result = octeon_i2c_wait(i2c);
    - if (result)
    - return result;

    - for (i = 0; i < length; i++) {
    + for (i = 0; i < length; ) {
    tmp = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_STAT);
    + result = octeon_i2c_lost_arb(tmp, !(ctl & TWSI_CTL_AAK));
    + if (result)
    + return result;

    - if ((tmp != STAT_RXDATA_ACK) && (tmp != STAT_RXADDR_ACK)) {
    - dev_err(i2c->dev,
    - "%s: bad status before read (0x%x)\n",
    - __func__, tmp);
    - return -EIO;
    + switch (tmp) {
    + case STAT_RXDATA_ACK:
    + case STAT_RXDATA_NAK:
    + data[i++] = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_DATA);
    }

    - if (i + 1 < length)
    - octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
    - TWSI_CTL_ENAB | TWSI_CTL_AAK);
    - else
    - octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
    - TWSI_CTL_ENAB);
    + /* NAK last recv'd byte, as a no-more-please */
    + if (last && i == length - 1)
    + ctl &= ~TWSI_CTL_AAK;

    + /* clr iflg to allow next event */
    + octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL, ctl);
    result = octeon_i2c_wait(i2c);
    if (result)
    return result;

    - data[i] = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_DATA);
    if (recv_len && i == 0) {
    if (data[i] > I2C_SMBUS_BLOCK_MAX + 1) {
    dev_err(i2c->dev,
    @@ -457,6 +567,7 @@ static int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,

    for (i = 0; ret == 0 && i < num; i++) {
    struct i2c_msg *pmsg = &msgs[i];
    + bool last = (i == (num - 1));

    dev_dbg(i2c->dev,
    "Doing %s %d byte(s) to/from 0x%02x - %d of %d messages\n",
    @@ -464,10 +575,11 @@ static int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
    pmsg->len, pmsg->addr, i + 1, num);
    if (pmsg->flags & I2C_M_RD)
    ret = octeon_i2c_read(i2c, pmsg->addr, pmsg->buf,
    - &pmsg->len, pmsg->flags & I2C_M_RECV_LEN);
    + &pmsg->len, !i, last,
    + pmsg->flags & I2C_M_RECV_LEN);
    else
    ret = octeon_i2c_write(i2c, pmsg->addr, pmsg->buf,
    - pmsg->len);
    + pmsg->len, !i, last);
    }
    octeon_i2c_stop(i2c);

    --
    1.9.1
    \
     
     \ /
      Last update: 2016-03-31 13:01    [W:3.408 / U:0.340 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site