[3/3] i2c: exynos5: do not check TRANS_STATUS in case of Exynos7 variant

Message ID 20171130143007.30258-4-a.hajda@samsung.com
State Changes Requested
Headers show
Series
  • i2c: exynos5: bus recovery implementation
Related show

Commit Message

Andrzej Hajda Nov. 30, 2017, 2:30 p.m.
HSI2C_MASTER_ST_LOSE state is not documented properly, extensive tests
show that hardware is usually able to recover from this state without
interrupting the transfer. On the other side enforcing transfer repetition
in such case does not help in many situations, especially on busy systems
and causes -EAGAIN and -ETIMEOUT errors. Moreover documentation says that
such state can be caused by slave clock stretching, and should not be treated
as an error.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/i2c/busses/i2c-exynos5.c | 6 ------
 1 file changed, 6 deletions(-)

Comments

Wolfram Sang Jan. 15, 2018, 8:53 p.m. | #1
On Thu, Nov 30, 2017 at 03:30:07PM +0100, Andrzej Hajda wrote:
> HSI2C_MASTER_ST_LOSE state is not documented properly, extensive tests
> show that hardware is usually able to recover from this state without
> interrupting the transfer. On the other side enforcing transfer repetition
> in such case does not help in many situations, especially on busy systems
> and causes -EAGAIN and -ETIMEOUT errors. Moreover documentation says that
> such state can be caused by slave clock stretching, and should not be treated
> as an error.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Can this be applied independently of my comments to patch 2?
Andrzej Hajda Jan. 16, 2018, 9:40 a.m. | #2
On 15.01.2018 21:53, Wolfram Sang wrote:
> On Thu, Nov 30, 2017 at 03:30:07PM +0100, Andrzej Hajda wrote:
>> HSI2C_MASTER_ST_LOSE state is not documented properly, extensive tests
>> show that hardware is usually able to recover from this state without
>> interrupting the transfer. On the other side enforcing transfer repetition
>> in such case does not help in many situations, especially on busy systems
>> and causes -EAGAIN and -ETIMEOUT errors. Moreover documentation says that
>> such state can be caused by slave clock stretching, and should not be treated
>> as an error.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Can this be applied independently of my comments to patch 2?
>
Yes, please apply it alone. I will continue work on patch 2.

--
Regards
Andrzej
Wolfram Sang Jan. 17, 2018, 11:23 p.m. | #3
On Tue, Jan 16, 2018 at 10:40:36AM +0100, Andrzej Hajda wrote:
> On 15.01.2018 21:53, Wolfram Sang wrote:
> > On Thu, Nov 30, 2017 at 03:30:07PM +0100, Andrzej Hajda wrote:
> >> HSI2C_MASTER_ST_LOSE state is not documented properly, extensive tests
> >> show that hardware is usually able to recover from this state without
> >> interrupting the transfer. On the other side enforcing transfer repetition
> >> in such case does not help in many situations, especially on busy systems
> >> and causes -EAGAIN and -ETIMEOUT errors. Moreover documentation says that
> >> such state can be caused by slave clock stretching, and should not be treated
> >> as an error.
> >>
> >> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> > Can this be applied independently of my comments to patch 2?
> >
> Yes, please apply it alone. I will continue work on patch 2.

I just thought it might be nice to have a comment where you removed the
code summarizing your findings. So we will remember about this in the
future. Makes sense?

Patch

diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
index 4ca43980e2ed..d579b45b3092 100644
--- a/drivers/i2c/busses/i2c-exynos5.c
+++ b/drivers/i2c/busses/i2c-exynos5.c
@@ -445,12 +445,6 @@  static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id)
 			i2c->state = -ETIMEDOUT;
 			goto stop;
 		}
-
-		trans_status = readl(i2c->regs + HSI2C_TRANS_STATUS);
-		if ((trans_status & HSI2C_MASTER_ST_MASK) == HSI2C_MASTER_ST_LOSE) {
-			i2c->state = -EAGAIN;
-			goto stop;
-		}
 	} else if (int_status & HSI2C_INT_I2C) {
 		trans_status = readl(i2c->regs + HSI2C_TRANS_STATUS);
 		if (trans_status & HSI2C_NO_DEV_ACK) {