lkml.org 
[lkml]   [2017]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.12 133/142] i2c: at91: manage unexpected RXRDY flag when starting a transfer
    Date
    From: Ludovic Desroches <ludovic.desroches@atmel.com>

    3.12-stable review patch. If anyone has any objections, please let me know.

    ===============

    commit a9bed6b10bd117a300cceb9062003f7a2761ef99 upstream.

    In some cases, we could start a new i2c transfer with the RXRDY flag
    set. It is not a clean state and it leads to print annoying error
    messages even if there no real issue. The cause is only having garbage
    data in the Receive Holding Register because of a weird behavior of the
    RXRDY flag.

    Reported-by: Peter Rosin <peda@lysator.liu.se>
    Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
    Tested-by: Peter Rosin <peda@lysator.liu.se>
    Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
    Fixes: 93563a6a71bb ("i2c: at91: fix a race condition when using the DMA controller")
    Signed-off-by: Jiri Slaby <jslaby@suse.cz>
    ---
    drivers/i2c/busses/i2c-at91.c | 36 ++++++++++++++++++++++++++----------
    1 file changed, 26 insertions(+), 10 deletions(-)

    diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
    index e6f18b241255..70782d560407 100644
    --- a/drivers/i2c/busses/i2c-at91.c
    +++ b/drivers/i2c/busses/i2c-at91.c
    @@ -272,8 +272,14 @@ error:

    static void at91_twi_read_next_byte(struct at91_twi_dev *dev)
    {
    - if (dev->buf_len <= 0)
    + /*
    + * If we are in this case, it means there is garbage data in RHR, so
    + * delete them.
    + */
    + if (!dev->buf_len) {
    + at91_twi_read(dev, AT91_TWI_RHR);
    return;
    + }

    *dev->buf = at91_twi_read(dev, AT91_TWI_RHR) & 0xff;
    --dev->buf_len;
    @@ -370,6 +376,24 @@ static irqreturn_t atmel_twi_interrupt(int irq, void *dev_id)

    if (!irqstatus)
    return IRQ_NONE;
    + /*
    + * In reception, the behavior of the twi device (before sama5d2) is
    + * weird. There is some magic about RXRDY flag! When a data has been
    + * almost received, the reception of a new one is anticipated if there
    + * is no stop command to send. That is the reason why ask for sending
    + * the stop command not on the last data but on the second last one.
    + *
    + * Unfortunately, we could still have the RXRDY flag set even if the
    + * transfer is done and we have read the last data. It might happen
    + * when the i2c slave device sends too quickly data after receiving the
    + * ack from the master. The data has been almost received before having
    + * the order to send stop. In this case, sending the stop command could
    + * cause a RXRDY interrupt with a TXCOMP one. It is better to manage
    + * the RXRDY interrupt first in order to not keep garbage data in the
    + * Receive Holding Register for the next transfer.
    + */
    + if (irqstatus & AT91_TWI_RXRDY)
    + at91_twi_read_next_byte(dev);

    /*
    * When a NACK condition is detected, the I2C controller sets the NACK,
    @@ -412,8 +436,6 @@ static irqreturn_t atmel_twi_interrupt(int irq, void *dev_id)
    if (irqstatus & (AT91_TWI_TXCOMP | AT91_TWI_NACK)) {
    at91_disable_twi_interrupts(dev);
    complete(&dev->cmd_complete);
    - } else if (irqstatus & AT91_TWI_RXRDY) {
    - at91_twi_read_next_byte(dev);
    } else if (irqstatus & AT91_TWI_TXRDY) {
    at91_twi_write_next_byte(dev);
    }
    @@ -428,7 +450,6 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
    {
    int ret;
    bool has_unre_flag = dev->pdata->has_unre_flag;
    - unsigned sr;

    /*
    * WARNING: the TXCOMP bit in the Status Register is NOT a clear on
    @@ -465,7 +486,7 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
    dev->transfer_status = 0;

    /* Clear pending interrupts, such as NACK. */
    - sr = at91_twi_read(dev, AT91_TWI_SR);
    + at91_twi_read(dev, AT91_TWI_SR);

    if (!dev->buf_len) {
    at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_QUICK);
    @@ -473,11 +494,6 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
    } else if (dev->msg->flags & I2C_M_RD) {
    unsigned start_flags = AT91_TWI_START;

    - if (sr & AT91_TWI_RXRDY) {
    - dev_err(dev->dev, "RXRDY still set!");
    - at91_twi_read(dev, AT91_TWI_RHR);
    - }
    -
    /* if only one byte is to be read, immediately stop transfer */
    if (dev->buf_len <= 1 && !(dev->msg->flags & I2C_M_RECV_LEN))
    start_flags |= AT91_TWI_STOP;
    --
    2.12.2
    \
     
     \ /
      Last update: 2017-04-10 17:35    [W:4.117 / U:0.288 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site