Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752843AbdFGXEB (ORCPT ); Wed, 7 Jun 2017 19:04:01 -0400 Received: from wtarreau.pck.nerim.net ([62.212.114.60]:50489 "EHLO 1wt.eu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752792AbdFGXD5 (ORCPT ); Wed, 7 Jun 2017 19:03:57 -0400 From: Willy Tarreau To: linux-kernel@vger.kernel.org, stable@vger.kernel.org, linux@roeck-us.net Cc: Ludovic Desroches , Wolfram Sang , Jiri Slaby , Willy Tarreau Subject: [PATCH 3.10 198/250] i2c: at91: manage unexpected RXRDY flag when starting a transfer Date: Thu, 8 Jun 2017 00:59:44 +0200 Message-Id: <1496876436-32402-199-git-send-email-w@1wt.eu> X-Mailer: git-send-email 2.8.0.rc2.1.gbe9624a In-Reply-To: <1496876436-32402-1-git-send-email-w@1wt.eu> References: <1496876436-32402-1-git-send-email-w@1wt.eu> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4039 Lines: 106 From: Ludovic Desroches 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 Signed-off-by: Ludovic Desroches Tested-by: Peter Rosin Signed-off-by: Wolfram Sang Fixes: 93563a6a71bb ("i2c: at91: fix a race condition when using the DMA controller") Signed-off-by: Jiri Slaby Signed-off-by: Willy Tarreau --- 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 c880d13..f079877 100644 --- a/drivers/i2c/busses/i2c-at91.c +++ b/drivers/i2c/busses/i2c-at91.c @@ -273,8 +273,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; @@ -371,6 +377,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, @@ -413,8 +437,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); } @@ -429,7 +451,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 @@ -466,7 +487,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); @@ -474,11 +495,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.8.0.rc2.1.gbe9624a