Patch "i2c: at91: fix write transfers by clearing pending interrupt first" has been added to the 4.3-stable tree
This is a note to let you know that I've just added the patch titled
i2c: at91: fix write transfers by clearing pending interrupt first
to the 4.3-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
i2c-at91-fix-write-transfers-by-clearing-pending-interrupt-first.patch
and it can be found in the queue-4.3 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
Quoted 1 line
From
6f6ddbb09d2a5baded0e23add3ad2d9e9417ab30Mon Sep 17 00:00:00 2001
From: Cyrille Pitchen <[email protected]>
Date: Wed, 21 Oct 2015 15:44:03 +0200
Subject: i2c: at91: fix write transfers by clearing pending interrupt first
From: Cyrille Pitchen <[email protected]>
commit 6f6ddbb09d2a5baded0e23add3ad2d9e9417ab30 upstream.
In some cases a NACK interrupt may be pending in the Status Register (SR) as a result of a previous transfer. However at91_do_twi_transfer() did not read the SR to clear pending interruptions before starting a new transfer. Hence a NACK interrupt rose as soon as it was enabled again at the I2C controller level, resulting in a wrong sequence of operations and strange patterns of behaviour on the I2C bus, such as a clock stretch followed by a restart of the transfer.
This first issue occurred with both DMA and PIO write transfers.
Also when a NACK error was detected during a PIO write transfer, the interrupt handler used to wrongly start a new transfer by writing into the Transmit Holding Register (THR). Then the I2C slave was likely to reply with a second NACK.
This second issue is fixed in atmel_twi_interrupt() by handling the TXRDY status bit only if both the TXCOMP and NACK status bits are cleared.
Tested with a at24 eeprom on sama5d36ek board running a linux-4.1-at91 kernel image. Adapted to linux-next.
--- drivers/i2c/busses/i2c-at91.c | 58 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 50 insertions(+), 8 deletions(-)
drivers/i2c/busses/i2c-at91.c +50 -8
@@ -465,19 +465,57 @@ static irqreturn_t atmel_twi_interrupt(iif (!irqstatus)return IRQ_NONE ; -else if (irqstatus &AT91_TWI_RXRDY ) -at91_twi_read_next_byte (dev); -else if (irqstatus &AT91_TWI_TXRDY ) -at91_twi_write_next_byte (dev); - -/* catch error flags */ - dev->transfer_status |= status; +/* +* When a NACK condition is detected, the I2C controller sets the NACK, +* TXCOMP and TXRDY bits all together in the Status Register (SR). +* +* 1 - Handling NACK errors with CPU write transfer. +* +* In such case, we should not write the next byte into the Transmit +* Holding Register (THR) otherwise the I2C controller would start a new +* transfer and the I2C slave is likely to reply by another NACK. +* +* 2 - Handling NACK errors with DMA write transfer. +* +* By setting the TXRDY bit in the SR, the I2C controller also triggers +* the DMA controller to write the next data into the THR. Then the +* result depends on the hardware version of the I2C controller. +* +* 2a - Without support of the Alternative Command mode. +* +* This is the worst case: the DMA controller is triggered to write the +* next data into the THR, hence starting a new transfer: the I2C slave +* is likely to reply by another NACK. +* Concurrently, this interrupt handler is likely to be called to manage +* the first NACK before the I2C controller detects the second NACK and +* sets once again the NACK bit into the SR. +* When handling the first NACK, this interrupt handler disables the I2C +* controller interruptions, especially the NACK interrupt. +* Hence, the NACK bit is pending into the SR. This is why we should +* read the SR to clear all pending interrupts at the beginning of +* at91_do_twi_transfer() before actually starting a new transfer. +* +* 2b - With support of the Alternative Command mode. +* +* When a NACK condition is detected, the I2C controller also locks the +* THR (and sets the LOCK bit in the SR): even though the DMA controller +* is triggered by the TXRDY bit to write the next data into the THR, +* this data actually won't go on the I2C bus hence a second NACK is not +* generated. +*/ 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); } +/* catch error flags */ + dev->transfer_status |= status; +return IRQ_HANDLED ; }
@@ -487,6 +525,7 @@ static int at91_do_twi_transfer(struct aunsigned long time_left;bool has_unre_flag = dev->pdata->has_unre_flag;bool has_alt_cmd = dev->pdata->has_alt_cmd; +unsigned sr;/* * WARNING: the TXCOMP bit in the Status Register is NOT a clear on
@@ -537,6 +576,9 @@ static int at91_do_twi_transfer(struct areinit_completion (&dev->cmd_complete); dev->transfer_status =0 ; +/* Clear pending interrupts, such as NACK. */ + sr =at91_twi_read (dev,AT91_TWI_SR ); +if (dev->fifo_size) {unsigned fifo_mr =at91_twi_read (dev,AT91_TWI_FMR );
@@ -558,7 +600,7 @@ static int at91_do_twi_transfer(struct a }else if (dev->msg->flags &I2C_M_RD ) {unsigned start_flags =AT91_TWI_START ; -if (at91_twi_read (dev,AT91_TWI_SR ) &AT91_TWI_RXRDY ) { +if (sr &AT91_TWI_RXRDY ) {dev_err (dev->dev,"RXRDY still set!" );at91_twi_read (dev,AT91_TWI_RHR ); }
Patches currently in stable-queue which might be from [email protected] are
queue-4.3/i2c-at91-fix-write-transfers-by-clearing-pending-interrupt-first.patch