From 27caca9d2e01c92b26d0690f065aad093fea01c7 Mon Sep 17 00:00:00 2001 From: Alexander Kochetkov Date: Tue, 18 Nov 2014 21:00:58 +0400 Subject: i2c: omap: fix NACK and Arbitration Lost irq handling commit 1d7afc95946487945cc7f5019b41255b72224b70 (i2c: omap: ack IRQ in parts) changed the interrupt handler to complete transfers without clearing XRDY (AL case) and ARDY (NACK case) flags. XRDY or ARDY interrupts will be fired again. As a result, ISR keep processing transfer after it was already complete (from the driver code point of view). A didn't see real impacts of the 1d7afc9, but it is really bad idea to have ISR running on user data after transfer was complete. It looks, what 1d7afc9 violate TI specs in what how AL and NACK should be handled (see Note 1, sprugn4r, Figure 17-31 and Figure 17-32). According to specs (if I understood correctly), in case of NACK and AL driver must reset NACK, AL, ARDY, RDR, and RRDY (Master Receive Mode), and NACK, AL, ARDY, and XDR (Master Transmitter Mode). All that is done down the code under the if condition: if (stat & (OMAP_I2C_STAT_ARDY | OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) ... The patch restore pre 1d7afc9 logic of handling NACK and AL interrupts, so no interrupts is fired after ISR informs the rest of driver what transfer complete. Note: instead of removing break under NACK case, we could just replace 'break' with 'continue' and allow NACK transfer to finish using ARDY event. I found that NACK and ARDY bits usually set together. That case confirm TI wiki: http://processors.wiki.ti.com/index.php/I2C_Tips#Detecting_and_handling_NACK In order if someone interested in the event traces for NACK and AL cases, I sent them to mailing list. Tested on Beagleboard XM C. Signed-off-by: Alexander Kochetkov Fixes: 1d7afc9 i2c: omap: ack IRQ in parts Cc: # v3.7+ Acked-by: Felipe Balbi Tested-by: Aaro Koskinen Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-omap.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers') diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 26942c159de1..32dc65183ea8 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -922,14 +922,12 @@ omap_i2c_isr_thread(int this_irq, void *dev_id) if (stat & OMAP_I2C_STAT_NACK) { err |= OMAP_I2C_STAT_NACK; omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK); - break; } if (stat & OMAP_I2C_STAT_AL) { dev_err(dev->dev, "Arbitration lost\n"); err |= OMAP_I2C_STAT_AL; omap_i2c_ack_stat(dev, OMAP_I2C_STAT_AL); - break; } /* -- cgit v1.2.3 From d39f77b06a712fcba6185a20bb209e357923d980 Mon Sep 17 00:00:00 2001 From: Andrew Jackson Date: Fri, 7 Nov 2014 12:10:44 +0000 Subject: i2c: designware: prevent early stop on TX FIFO empty If the Designware core is configured with IC_EMPTYFIFO_HOLD_MASTER_EN set to zero, allowing the TX FIFO to become empty causes a STOP condition to be generated on the I2C bus. If the transmit FIFO threshold is set too high, an erroneous STOP condition can be generated on long transfers - particularly where the interrupt latency is extended. Signed-off-by: Andrew Jackson Signed-off-by: Liviu Dudau Tested-by: Mika Westerberg Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-designware-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c index edca99dbba23..23628b7bfb8d 100644 --- a/drivers/i2c/busses/i2c-designware-core.c +++ b/drivers/i2c/busses/i2c-designware-core.c @@ -359,7 +359,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev) } /* Configure Tx/Rx FIFO threshold levels */ - dw_writel(dev, dev->tx_fifo_depth - 1, DW_IC_TX_TL); + dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL); dw_writel(dev, 0, DW_IC_RX_TL); /* configure the i2c master */ -- cgit v1.2.3 From ccfc866356674cb3a61829d239c685af6e85f197 Mon Sep 17 00:00:00 2001 From: Alexander Kochetkov Date: Fri, 21 Nov 2014 04:16:51 +0400 Subject: i2c: omap: fix i207 errata handling commit 6d9939f651419a63e091105663821f9c7d3fec37 (i2c: omap: split out [XR]DR and [XR]RDY) changed the way how errata i207 (I2C: RDR Flag May Be Incorrectly Set) get handled. 6d9939f6514 code doesn't correspond to workaround provided by errata. According to errata ISR must filter out spurious RDR before data read not after. ISR must read RXSTAT to get number of bytes available to read. Because RDR could be set while there could no data in the receive FIFO. Restored pre 6d9939f6514 way of handling errata. Found by code review. Real impact haven't seen. Tested on Beagleboard XM C. Signed-off-by: Alexander Kochetkov Fixes: 6d9939f651419a63e09110 i2c: omap: split out [XR]DR and [XR]RDY Tested-by: Felipe Balbi Reviewed-by: Felipe Balbi Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-omap.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 32dc65183ea8..277a2288d4a8 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -952,11 +952,13 @@ omap_i2c_isr_thread(int this_irq, void *dev_id) if (dev->fifo_size) num_bytes = dev->buf_len; - omap_i2c_receive_data(dev, num_bytes, true); - - if (dev->errata & I2C_OMAP_ERRATA_I207) + if (dev->errata & I2C_OMAP_ERRATA_I207) { i2c_omap_errata_i207(dev, stat); + num_bytes = (omap_i2c_read_reg(dev, + OMAP_I2C_BUFSTAT_REG) >> 8) & 0x3F; + } + omap_i2c_receive_data(dev, num_bytes, true); omap_i2c_ack_stat(dev, OMAP_I2C_STAT_RDR); continue; } -- cgit v1.2.3